[llvm-commits] [llvm-gcc-4.2] r49171 - in /llvm-gcc-4.2/trunk/gcc: llvm-convert.cpp llvm-types.cpp

[Chris Lattner]

On Apr 5, 2008, at 12:55 AM, Duncan Sands wrote:

> Hi Dale, I disagree but I agree that it's not clear.  Maybe Chris
> can comment.  Chris: Dale modified llvm-gcc so that, when compiling
> without -fexceptions, it sticks "nounwind" on (1) calls [this is  
> fine],
> (2) function bodies [this is fine] and (3) function declarations  
> [which
> I disagree with].  The problematic example is:

Ok.  I'm not sure why #3 is bad.  If the external function ever did  
throw, badness would clearly happen :)

>> Consider compiling the following [module 1] without -fexceptions:
>>
>> extern void g(bool Allowed_To_Throw);
>> void f(void) {
>> g(false);
>> }
>>
>> and compiling the following [module 2] with -fexceptions:
>>
>> void g(bool Allowed_To_Throw) {
>> ...
>> if (Allowed_To_Throw)
>>   throw something;
>> else
>>   abort();
>> ...
>> }
>>
>> then linking the two modules together.  Here g is a function that
>> doesn't throw exceptions unless Allowed_To_Throw is true.  With your
>> patch g will be declared nounwind in the first module but not in the
>> second.  When you link them g may be marked nounwind in the linked
>> module which would be wrong (I don't know if this happens or not
>> currently, but it doesn't matter - we shouldn't rely on this kind of
>> linker detail for correctness).
>
> My take is that compiling with !flag_exceptions tells you something
> about the functions in this module but nothing about functions not
> in this module, so marking external functions nounwind is wrong.
> (The fact that calls to them from this module do not throw is taken
> care of by marking *calls* nounwind, which we now do).

Right.  However, I think that this should be handled in the linker.   
When llvm-link'ing together a function prototype with anything else (a  
prototype or definition) the possibility of throwing should always win  
(in other words "resultnothrow = proto1nothrow & proto2nothrow").  If  
we did this, I don't think there would be a problem.

>> I disagree; this is exactly the sort of detail where the linker
>> behavior should be specified.
>
> I agree that linker behaviour with respect to attribute conflicts
> could be better specified.  At a minimum attributes on a definition
> should overcome attributes on a declaration.

I don't think that is sufficient though, prototypes that are  
"throwable" in one translation unit should make the destination  
prototype "throwable" even if there is no body.  However, if the body  
is available, and the body is marked nothrow, that should override any  
prototypes.

>> In fact, if g throws and its caller f
>> does not expect it to, that is probably a mistake, and one the linker
>> could usefully tell the user about.  (I see that it is not a mistake
>> in your example but I think a mistake is much more likely.)
>
> It is perfectly possible (in fact normal) to have a body marked  
> nounwind
> and a declaration which is not.  That is because when the module  
> containing
> the body is optimized, prune-eh may set nounwind on the body; while  
> in the
> other module there's no way you could know to put nounwind on the  
> declaration.

right.

> Suppose that when linking, attributes on the definition overcome  
> attributes on
> the declaration.  If that is done then it solves the problem if you  
> don't put
> nounwind on declarations (my suggestion).  But it is not enough if  
> you do put
> nounwind on declarations (your code) - then further linker logic is  
> definitely
> needed.  Consider the following:
>
> In a third module [module 3] (compiled with -fexceptions) you have:
>
> extern void g(bool Allowed_To_Throw);
> void h(void) {
>  try {
>    g(true);
>  } catch (...) {
>    printf_a_message;
>  }
> }
>
> Now link modules 1 and 3.  You have two declarations of g, one from  
> module 1
> marked nounwind, another from module 3 not marked nounwind.  This  
> might result
> in g being declared nounwind in the linked module.

No, two conflicting prototypes mean the dest prototype should be  
allowed to throw.

>> Right now running your example through llvm-ld does appear to mark g
>> as nounwind, even though it throws.  I believe the appropriate thing
>> to do is change the linker to take the nounwind bit from the
>> definition, and perhaps emit a warning.
>
> I don't think LLVM is into producing this kind of warning.

In general, emitting warnings from the optimizer is dangerous.  We do  
that in one case right now (when LTO finds a call to a function  
passing in arguments that the callee ends up not knowing about) and  
already have a confused end-user bug report asking "wtf??".

> Here's what I think:
>
> (1) declarations should not be marked nounwind just because this  
> module is
> being compiled without -fexceptions.
> (2) attributes on a function definition should over-ride any on a  
> declaration.

I tend to think that this should be handled in the linker.  Is there  
any reason we can't do that?

-Chris