Allow output constraints on "asm goto"

[void]

Summary

Remove the restrictions that preventing "asm goto" from returning non-void
values. The values returned by "asm goto" are only valid on the "fallthrough"
path.

[void]

Emit asm goto fallthrough early so that any value extracts will show up in the
fallthrough block and not directly after the callbr.

[void]

Adjust the ASM so that it references labels.

[void]

This change is ready for review. PTAL.

[jyknight]

I think -Wuninitialized (UninitializedValues.cpp) should be taught how to detect the use of output variables in error blocks, at least for trivial cases.

Actually, for some reason -- it looks like the warning is failing the wrong way right now, and emits an uninitialized use warning even where there shouldn't be one.

E.g. this example should be okay:

int foo(int x) {
  int y;
  asm goto("# %0 %1 %2" : "=r"(y) : "r"(x) : : err);
  return y;
err:
  return -1;
}

But now warns:

$ clang -Wall -fsyntax-only foo.c
foo.c:4:10: warning: variable 'y' is uninitialized when used here [-Wuninitialized]
  return y;
         ^
foo.c:2:8: note: initialize the variable 'y' to silence this warning
  int y;
       ^
        = 0
1 warning generated.

I'd expect a warning only if the code was modified to say "return y" in the err block.

[jyknight]

Also, since this means we are no longer simply implementing according to GCC's documentation, I think this means we'll need a brand new section in the Clang docs for its inline-asm support.

[void]

Analyze "asm goto" for initialized variables.

An "asm goto" statement is a terminator and thus doesn't indicate variable
assignments like normal inline assembly. Handle them specially.

[void]

In D69876#1740286, @jyknight wrote:

I think -Wuninitialized (UninitializedValues.cpp) should be taught how to detect the use of output variables in error blocks, at least for trivial cases.

Actually, for some reason -- it looks like the warning is failing the wrong way right now, and emits an uninitialized use warning even where there shouldn't be one.

Nice catch! I updated the patch with a fix. PTAL.

[void]

In D69876#1740289, @jyknight wrote:

Also, since this means we are no longer simply implementing according to GCC's documentation, I think this means we'll need a brand new section in the Clang docs for its inline-asm support.

Sure. Which file do we list such things in?

[nickdesaulniers]

In D69876#1741294, @void wrote:

In D69876#1740286, @jyknight wrote:

I think -Wuninitialized (UninitializedValues.cpp) should be taught how to detect the use of output variables in error blocks, at least for trivial cases.

Actually, for some reason -- it looks like the warning is failing the wrong way right now, and emits an uninitialized use warning even where there shouldn't be one.

Nice catch! I updated the patch with a fix. PTAL.

Please add a test for that!

[void]

the real update this time.

[void]

In D69876#1741381, @nickdesaulniers wrote:

Nice catch! I updated the patch with a fix. PTAL.

Please add a test for that!

I did, but the "arc" program pushed the wrong change. :-( Fixed.

When are we going to use GitHub's PRs?

[void]

Add blurb about asm goto with output constraints to the "language extensions"
documentation.

[void]

Adjust label references to account for in/out constraints.

[void]

Move adjustment before error check.

[void]

Friendly ping. :-)

[nickdesaulniers]

clang/lib/Analysis/UninitializedValues.cpp

830

GCCAsmStmt inherits from AsmStmt which has an outputs() method for returning an iterator. Please use that an a range based for loop.

875

Don't declare as as const, then you won't need this const_cast.

[void]

Remove a "const_cast" and use iterators for output constraints.

[void]

Changes made. PTAL

[nickdesaulniers]

clang/lib/Analysis/UninitializedValues.cpp

831

this is still not a range based for loop.

Does:

for (const auto &O : as->outputs())
  if (const VarDecl *VD = findVar(O).getDecl())
    vals[VD] = Initialized;

not work?

[void]

Adjust loop.

[void]

clang/lib/Analysis/UninitializedValues.cpp

831

Done.

[void]

Any further comments?

[rnk]

Changes seem fine to me, FWIW.
+@rsmith

clang/test/Analysis/uninit-asm-goto.cpp

10

This could use a test for the case where an input is uninitialized, and where an uninitialized value is used along the error path.

[void]

clang/test/Analysis/uninit-asm-goto.cpp

10

I have a follow-up patch that I'll upload soon that warns about uninitialized variable use on the indirect paths. I wanted to separate it to keep this patch size down. If you'd prefer I can just add it.