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

[Duncan Sands]

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:

> 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).

> 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.

> 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.

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.  Then if you re-optimize
the linked module, the function h will be simplified to:

void h(void) {
  g(true);
}

which is wrong.  Thus in the case of two declarations with different nounwind
markings, the linker would have to strip off nounwind.

This could be done, but I think the mistake was generating a wrong declaration
in the first place.

> Consider also that llvm-gcc cannot guarantee that all declarations and  
> definitions match.  If we do as you suggest, they will mismatch when  
> the definition is nounwind.
>
> As for the linker behavior, I would expect the definition to win.

Sure, this is the general rule.  This is why it doesn't matter if a
declaration is not nounwind but the definition is nounwind.  It had
better not matter because as I mentioned above this situation is
produced by prune-eh all the time!

> 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.

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.

Ciao,

Duncan.