[cfe-commits] r91073 - in /cfe/trunk: lib/Sema/SemaDecl.cpp test/Sema/return.c

[Mike Stump]

On Dec 10, 2009, at 5:59 PM, Chris Lattner wrote:
> On Dec 10, 2009, at 5:42 PM, Mike Stump wrote:
> 
>> On Dec 10, 2009, at 4:08 PM, Chris Lattner wrote:
>>> On Dec 10, 2009, at 2:57 PM, Mike Stump wrote:
>>> 
>>>> Author: mrs
>>>> Date: Thu Dec 10 16:57:48 2009
>>>> New Revision: 91073
>>>> 
>>>> URL: http://llvm.org/viewvc/llvm-project?rev=91073&view=rev
>>>> Log:
>>>> Don't complain about falling off the end of a function with an asm
>>>> block, if the function is supposed to return a value as we don't know
>>>> exactly what the asm code does.
>>> 
>>> Why?
>> 
>> Because people write code like:
>> 
>> void* t4(void) { __asm mov eax, fs:[0x10] }
> 
> That is not gnu asm syntax.

True.

> Also, you're not fixing the right thing here.  Several issues:
> 
> 1. Setting eax is not guaranteed to set the return value

Actually, yes, it is.

> 2. The bigger issue is that you're *changing the CFG* to avoid a *data* issue.

I'd put it this way, I'm fixing the AST to be more correct.  I'm not changing the CFG per se, I'm changing what I think I know based upon what is in the CFG.  Since we don't know that an asm block will actually flow out, or if it sets eax, we return a conservative answer from the analysis.  The answer we return is, we don't know what the asm code does exactly, which I'd claim, is exactly the right answer for now.

In the long run, if we want, we could analyze the asm code in detail, and improve what we think we know...

>> Also, int foo() { asm("a: jump a"); } doesn't fall off the end.  Essentially, trying to understand what is inside the asm is a dangerous thing.
> 
> You're right, which is why we should follow gcc's policy of assuming it can fall through!

It is annoying when

int foo() {
  asm xor eax, eax
}

gives a warning.

Bear in mind, gcc's policy for:

asm int foo() {
  xor eax, eax
}

is to _not_ give the warning.