Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

__attribute__ ((noreturn)) in a function parameter #2833

Closed
filcab opened this issue Jun 14, 2008 · 26 comments
Closed

__attribute__ ((noreturn)) in a function parameter #2833

filcab opened this issue Jun 14, 2008 · 26 comments
Labels
bugzilla Issues migrated from bugzilla clang:frontend Language frontend issues, e.g. anything involving "Sema"

Comments

@filcab
Copy link
Contributor

filcab commented Jun 14, 2008

Bugzilla Link 2461
Resolution FIXED
Resolved on Mar 12, 2010 00:57
Version unspecified
OS All
CC @mrvacbob,@d0k,@edwintorok,@efriedma-quic

Extended Description

clang doesn't allow attribute ((noreturn)) in a function parameter:
[filcab@farnsworth ~] $ cat b.c
extern void LibVEX_Init (
/* failure exit function */
attribute ((noreturn))
void (*failure_exit) ( void )
);
[filcab@farnsworth ~] $ ccc -o b.o -c b.c
clang -emit-llvm-bc -x c -o b.o b.c
b.c:3:20: warning: 'noreturn' attribute only applies to function types
attribute ((noreturn))
^
1 diagnostic generated.
[filcab@farnsworth ~] $ gcc -o b.o -c b.c
[filcab@farnsworth ~] $

@efriedma-quic
Copy link
Collaborator

Hmm, so it appears gcc allows applying the noreturn attribute to function pointers. This is going to be a bit complicated... clang currently doesn't support attaching function attributes to types.

A slightly more straightforward testcase:
typedef int (*a)();
typedef a b __attribute((noreturn));

@filcab
Copy link
Contributor Author

filcab commented Jun 15, 2008

I tried taking a peek at the source but I didn't see any way to know if a certain parameter is a function type or not. Is there a way to do that or is it necessary to make some more deep changes to know that?

Also: Is there any information about how clang works internally (except for the *.{cpp,h))?

@efriedma-quic
Copy link
Collaborator

I tried taking a peek at the source but I didn't see any way to know if a
certain parameter is a function type or not. Is there a way to do that or is it
necessary to make some more deep changes to know that?

I haven't checked this carefully, but I think what's happening is that by the time we try to apply the noreturn attribute, we no longer have a function, but instead a function pointer. That makes your testcase internally identical to mine.

The relevant place where clang handles declaration attributes is Sema::HandleDeclAttribute.

If you have a Decl, you can get its type by calling getType().

Unfortunately, the issue here is rather complicated, because it's really an architectural bug that function attributes are associated with the function decl rather than the function type.

Also: Is there any information about how clang works internally (except for the
*.{cpp,h))?

There's some limited documentation on the website (clang.llvm.org). Feel free to ask any questions, and additional documentation is always welcome.

@llvmbot
Copy link
Collaborator

llvmbot commented Jun 22, 2008

Another (possibly related) test case is

void f(int (attribute((stdcall)) *func)());

which gives

test.c:1:12: error: function cannot return array or function type 'int ()'
void f(int (attribute((stdcall)) *func)());
^

@lattner
Copy link
Collaborator

lattner commented Jun 29, 2008

I just completed a round of significant cleanups to attribute processing stuff. Implementing this now should be pretty easy (just change these "function attributes" to be type attributes). However, the hard part is that we have no place to store these. They really should go on the function type, but those are uniqued and it is unclear how these affect type compatibility.

@lattner
Copy link
Collaborator

lattner commented Jun 29, 2008

One other way to handle this is as a form of type qualifier like CVR or address spaces.

@efriedma-quic
Copy link
Collaborator

They really should go on the function type, but those
are uniqued and it is unclear how these affect type compatibility.

Yeah, that's kind of tricky... I'd suggest that for attributes like noreturn, which don't affect the calling convention, function types with and without the attribute should be compatible, and for attributes that affect the ABI (like stdcall), the function types should be incompatible. (This isn't really typesafe, but C fundamentally isn't typesafe in this respect anyway.)

Here's an interesting question (although I suppose it's only partly on-topic): suppose you have a pointer to a function returning a function pointer, and this declaration is decorated with __attribute((noreturn)). Which function is affected? And where would you place the attribute specifier to affect the other function?

@llvmbot
Copy link
Collaborator

llvmbot commented May 16, 2009

This appears to be fixed in trunk.

@efriedma-quic
Copy link
Collaborator

I wouldn't really call it fixed; we're simply ignoring it now.

@edwintorok
Copy link
Contributor

I wouldn't really call it fixed; we're simply ignoring it now.

It is not fixed, compilation fails with this error now:
priv/main/vex_util.c:221:1: error: function declared 'noreturn' should not return [-Winvalid-noreturn]

Since clang doesn't know that noreturn applies to function pointers, it doesn't know that vex_failure_exit is noreturn, and thinks it may return.

@nunoplopes
Copy link
Member

*** Bug llvm/llvm-bugzilla-archive#5402 has been marked as a duplicate of this bug. ***

@nunoplopes
Copy link
Member

*** Bug llvm/llvm-bugzilla-archive#5280 has been marked as a duplicate of this bug. ***

@nunoplopes
Copy link
Member

*** Bug llvm/llvm-bugzilla-archive#5578 has been marked as a duplicate of this bug. ***

@llvmbot
Copy link
Collaborator

llvmbot commented Dec 10, 2009

I noticed something interesting about clang's behavior in this case. If when you declare the function pointer, you put the noreturn attribute at the beginning of the declaration, clang warns:

$ cat t.c
attribute((noreturn)) void f(attribute((noreturn)) void (*x)(void))
{
x();
}

$ clang -c t.c
t.c:6:1: warning: function declared 'noreturn' should not return
[-Winvalid-noreturn]
}
^
1 diagnostic generated.
$

But when you put it at the end of the declaration, it doesn't warn:

$ cat t.c
attribute((noreturn)) void f(void (*x)(void) attribute((noreturn)))
{
x();
}

$ clang -c t.c
$

Correct me if I'm wrong, but I think that the noreturn is getting applied to the return type of the function pointer in the first case, instead of the function pointer itself. In the second case, clang is correctly applying noreturn to the function pointer type.

@llvmbot
Copy link
Collaborator

llvmbot commented Dec 10, 2009

Correct me if I'm wrong, but I think that the noreturn is getting applied to
the return type of the function pointer in the first case, instead of the
function pointer itself.
Or, the attribute for some strange reason is being silently ignored. It just occurred to me that if clang really did try to apply noreturn to the void type, it would have printed a warning to the effect of the attribute only applying to function types.

Now the question is: why is the attribute ignored when it's at the beginning of the decl but applied when it's at the end?

@llvmbot
Copy link
Collaborator

llvmbot commented Dec 11, 2009

Correct me if I'm wrong, but I think that the noreturn is getting applied to
the return type of the function pointer in the first case, instead of the
function pointer itself.
Or, the attribute for some strange reason is being silently ignored. It just
occurred to me that if clang really did try to apply noreturn to the void type,
it would have printed a warning to the effect of the attribute only applying to
function types.
No, I was right the first time. I took a look at SemaType.cpp and found that, when it encounters a noreturn applied to a non-function type, it just returns without issuing a warning. It just happens to work with functions because we still attach the attribute to the function decl.

Now the question becomes: why is clang trying to apply the noreturn attribute to the return type instead of to the function (pointer) type?

@llvmbot
Copy link
Collaborator

llvmbot commented Dec 14, 2009

OK, I think I've got this figured out. The reason clang tries to apply the attribute to the return type is simply because it hasn't seen the rest of the function pointer yet. So, I've written a patch that lets clang apply noreturn to function pointer decls, and adds support for this to Sema::CheckFallThrough. It works for my simple testcase, so it should work for you. I'm going to send it to cfe-commits shortly.

@llvmbot
Copy link
Collaborator

llvmbot commented Dec 15, 2009

OK, I think I've got this figured out. The reason clang tries to apply the
attribute to the return type is simply because it hasn't seen the rest of the
function pointer yet. So, I've written a patch that lets clang apply noreturn
to function pointer decls, and adds support for this to Sema::CheckFallThrough.
It works for my simple testcase, so it should work for you. I'm going to send
it to cfe-commits shortly.

Thank you Charles. I hope you can take a look at the other affected attributes (mainly stdcall) please ?

@llvmbot
Copy link
Collaborator

llvmbot commented Dec 15, 2009

Thank you Charles.
You're welcome.
I hope you can take a look at the other affected attributes
(mainly stdcall) please ?

Where do you think I'm headed next?

The whole reason I'm even on the CC list for this bug was that my original bug (which related to stdcall) got resolved as a dupe of this one. I'm thinking about reopening it. It's similar, but it's not quite the same as this bug.

I take it you're trying to compile Wine, too? (I saw your patch on wine-patches, so I know you're involved with them. Good to see a fellow Wine dev.) That's what got me interested in all this.

Anyway, my patch got committed (with test pending), so now I'm going to resolve this bug.

@llvmbot
Copy link
Collaborator

llvmbot commented Dec 15, 2009

Thank you Charles.
You're welcome.
I hope you can take a look at the other affected attributes
(mainly stdcall) please ?

Where do you think I'm headed next?

The whole reason I'm even on the CC list for this bug was that my original bug
(which related to stdcall) got resolved as a dupe of this one. I'm thinking
about reopening it. It's similar, but it's not quite the same as this bug.

Exactly, as I tried your patch as soon as I saw it, and tried compiling the stdcall testcase, but got the warning nevertheless.

I take it you're trying to compile Wine, too? (I saw your patch on
wine-patches, so I know you're involved with them. Good to see a fellow Wine
dev.) That's what got me interested in all this.

Yeah, more is coming hopefully.

Anyway, my patch got committed (with test pending), so now I'm going to resolve
this bug.

Okay. I've CCed myself into that duplicate (didn't check the dups before).

@lattner
Copy link
Collaborator

lattner commented Dec 15, 2009

Thank you for your work in this area guys! Feel free to reopen the dupe if it isn't fixed.

@nunoplopes
Copy link
Member

mentioned in issue llvm/llvm-bugzilla-archive#5280

2 similar comments
@llvmbot
Copy link
Collaborator

llvmbot commented Nov 27, 2021

mentioned in issue llvm/llvm-bugzilla-archive#5280

@DougGregor
Copy link
Contributor

mentioned in issue llvm/llvm-bugzilla-archive#5280

@nunoplopes
Copy link
Member

mentioned in issue llvm/llvm-bugzilla-archive#5402

@nunoplopes
Copy link
Member

mentioned in issue llvm/llvm-bugzilla-archive#5578

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 3, 2021
cyndyishida pushed a commit to cyndyishida/llvm-project that referenced this issue Jun 17, 2022
…8be389a4dd7fd71fc9fa9c

[lldb] Fix 'r' and 'run' aliases on Apple Silicon
kitano-metro pushed a commit to RIKEN-RCCS/llvm-project that referenced this issue Jul 14, 2023
kitano-metro pushed a commit to RIKEN-RCCS/llvm-project that referenced this issue Jul 14, 2023
kitano-metro pushed a commit to RIKEN-RCCS/llvm-project that referenced this issue Jul 14, 2023
MasuhiroYamada pushed a commit to RIKEN-RCCS/llvm-project that referenced this issue Jul 28, 2023
MasuhiroYamada added a commit to RIKEN-RCCS/llvm-project that referenced this issue Jul 28, 2023
Feature/2937 apply llvm#1639, llvm#2833

See merge request a64fx-swpl/llvm-project!142
MasuhiroYamada pushed a commit to RIKEN-RCCS/llvm-project that referenced this issue Nov 22, 2023
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugzilla Issues migrated from bugzilla clang:frontend Language frontend issues, e.g. anything involving "Sema"
Projects
None yet
Development

No branches or pull requests

7 participants