[clang][IFS] Prevent Clang-IFS from Leaking symbols from inside a block.

[plotfi]

Summary

Built libdispatch with clang interface stubs. Ran into some block related issues. Basically VarDecl symbols can leak out because I wasn't checking the case where a VarDecl is contained inside a BlockDecl (versus a method or function).

This patch checks that a VarDecl is not a child decl of a BlockDecl.
This patch also does something very similar for c++ lambdas as well.

[compnerd]

Should probably add a check for __block variables.

[plotfi]

In D71301#1778834, @compnerd wrote:

Should probably add a check for __block variables.

I looked into it, I am not sure if anything special needs to be done for block. ie:

echo "void f() { __block int x = 42; void (^sum)(int) = ^(int y) { x = x + y; }; }" | clang -x c -fblocks -o - - -c  | llvm-nm -
                 U _Block_object_assignecho "void f() { __block int x = 42; void (^sum)(int) = ^(int y) { x = x + y; }; }" | clang -x c -fblocks -o - - -c  | llvm-nm -
                 U _Block_object_assign
                 U _Block_object_dispose
                 U _NSConcreteStackBlock
0000000000000000 r __block_descriptor_tmp
00000000000000c0 W __copy_helper_block_8_32r
00000000000000f0 W __destroy_helper_block_8_32r
0000000000000090 t __f_block_invoke
0000000000000000 T 
                 U _Block_object_dispose
                 U _NSConcreteStackBlock
0000000000000000 r __block_descriptor_tmp
00000000000000c0 W __copy_helper_block_8_32r
00000000000000f0 W __destroy_helper_block_8_32r
0000000000000090 t __f_block_invoke
0000000000000000 T

[compnerd]

clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp

60

Use isa rather than dyn_cast_or_null. Furthermore, the dyn_cast_or_null is wrong - Parent is guaranteed to be !NULL given the condition on L56.

if (const auto *VD = dyn_cast<VarDecl>(ND))
  if (const auto *P = VD->getParentFunctionOrMethod())
    if (isa<BlockDecl>(P) || isa<CXXMethodDecl>(P))
      return true;

[plotfi]

using isa

[plotfi]

Going to do an NFC pre-commit before this to add the braces.

[thakis]

This breaks tests on macOS: http://45.33.8.238/mac/5672/step_7.txt (macOS prefixes functions with an extra _.)

[plotfi]

In D71301#1817926, @thakis wrote:

This breaks tests on macOS: http://45.33.8.238/mac/5672/step_7.txt (macOS prefixes functions with an extra _.)

Thank You Nico! Will fix

[plotfi]

In D71301#1817926, @thakis wrote:

This breaks tests on macOS: http://45.33.8.238/mac/5672/step_7.txt (macOS prefixes functions with an extra _.)

Should be fixed now. Sorry about this. Thanks Again Nico.

[thakis]

This test fails on Windows: http://45.33.8.238/win/5658/step_7.txt

Looks like this expects itanium mangling; maybe you want -triple %itanium_abi_triple?

[thakis]

(And thanks for the mac fix!)

[thakis]

In D71301#1818233, @thakis wrote:

This test fails on Windows: http://45.33.8.238/win/5658/step_7.txt

Looks like this expects itanium mangling; maybe you want -triple %itanium_abi_triple?

Since the bot's been red for a while now, I went ahead and landed that fix in 84baf123. The bot's green now, but please check if that's what the test wants to test or if this just papers over things.

[plotfi]

In D71301#1818571, @thakis wrote:

In D71301#1818233, @thakis wrote:

This test fails on Windows: http://45.33.8.238/win/5658/step_7.txt

Looks like this expects itanium mangling; maybe you want -triple %itanium_abi_triple?

Since the bot's been red for a while now, I went ahead and landed that fix in 84baf123. The bot's green now, but please check if that's what the test wants to test or if this just papers over things.

Thank you for this!

[plotfi]

In D71301#1818233, @thakis wrote:

This test fails on Windows: http://45.33.8.238/win/5658/step_7.txt

Looks like this expects itanium mangling; maybe you want -triple %itanium_abi_triple?

I missed this message. Sorry about that. I'll remember to use %itanium_abi_triple in the future (didn't know that was a thing, was just using the linux triple if I wanted itanium abi). Thanks again.

[thegameg]

Hi @plotfi, this seems to cause failures with ASAN and UBSAN on green dragon: http://green.lab.llvm.org/green/job/clang-stage2-cmake-RgSan/6886/consoleFull

The following tests are failing:

Clang :: InterfaceStubs/driver-test.c
Clang :: InterfaceStubs/driver-test2.c

[plotfi]

Thanks for pinging on this @thegameg. I had been looking at this a day or two ago but got side-tracked. Will take another look.

In D71301#1826928, @thegameg wrote:

Hi @plotfi, this seems to cause failures with ASAN and UBSAN on green dragon: http://green.lab.llvm.org/green/job/clang-stage2-cmake-RgSan/6886/consoleFull

The following tests are failing:

Clang :: InterfaceStubs/driver-test.c
Clang :: InterfaceStubs/driver-test2.c

[plotfi]

This appears to only happen with -fintegrated-cc1, with fno-integrated-cc1 it does not repro. Still looking into this.

In D71301#1826928, @thegameg wrote:

Hi @plotfi, this seems to cause failures with ASAN and UBSAN on green dragon: http://green.lab.llvm.org/green/job/clang-stage2-cmake-RgSan/6886/consoleFull

The following tests are failing:

Clang :: InterfaceStubs/driver-test.c
Clang :: InterfaceStubs/driver-test2.c

[plotfi]

So, the culprit appears to be D69825. The way InterfaceStubs assembles the pipeline appears to trigger an asan bug. For now I will alter the tests to get the bots green, but once I get my bugzilla access sorted I intend to file a bug. -PL

In D71301#1826928, @thegameg wrote:

Hi @plotfi, this seems to cause failures with ASAN and UBSAN on green dragon: http://green.lab.llvm.org/green/job/clang-stage2-cmake-RgSan/6886/consoleFull

The following tests are failing:

Clang :: InterfaceStubs/driver-test.c
Clang :: InterfaceStubs/driver-test2.c

[thegameg]

In D71301#1832801, @plotfi wrote:

So, the culprit appears to be D69825. The way InterfaceStubs assembles the pipeline appears to trigger an asan bug. For now I will alter the tests to get the bots green, but once I get my bugzilla access sorted I intend to file a bug. -PL

In D71301#1826928, @thegameg wrote:

Hi @plotfi, this seems to cause failures with ASAN and UBSAN on green dragon: http://green.lab.llvm.org/green/job/clang-stage2-cmake-RgSan/6886/consoleFull

The following tests are failing:

Clang :: InterfaceStubs/driver-test.c
Clang :: InterfaceStubs/driver-test2.c

Great, thanks for taking a look!

clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp

auto isVisible = [](const NamedDecl *ND) -> bool {
       return ND->getVisibility() == DefaultVisibility;
     };
 
     auto ignoreDecl = [this, isVisible](const NamedDecl *ND) -> bool {
       if (!isVisible(ND))
         return true;
 
       if (const VarDecl *VD = dyn_cast<VarDecl>(ND)) {
+        if (const auto *Parent = VD->getParentFunctionOrMethod())
+          if (isa<BlockDecl>(Parent) || isa<CXXMethodDecl>(Parent))
+            return true;
+
         if ((VD->getStorageClass() == StorageClass::SC_Extern) ||
             (VD->getStorageClass() == StorageClass::SC_Static &&
              VD->getParentFunctionOrMethod() == nullptr))
           return true;
       }
 
       if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(ND)) {
         if (FD->isInlined() && !isa<CXXMethodDecl>(FD) &&
             !Instance.getLangOpts().GNUInline)

clang/test/InterfaceStubs/blocks.c

+// RUN: %clang_cc1 -emit-interface-stubs -fblocks -o - %s | FileCheck %s
+
+// CHECK: --- !experimental-ifs-v1
+// CHECK-NEXT: IfsVersion: 1.0
+// CHECK-NEXT: Triple:
+// CHECK-NEXT: ObjectFileFormat: ELF
+// CHECK-NEXT: Symbols:
+// CHECK-NEXT: ...
+static void (^f)(void*) = ^(void* data) { int i; };

clang/test/InterfaceStubs/lambda.cpp

+// RUN: %clang_cc1 -emit-interface-stubs -o - %s | FileCheck %s
+
+// CHECK: --- !experimental-ifs-v1
+// CHECK-NEXT: IfsVersion: 1.0
+// CHECK-NEXT: Triple:
+// CHECK-NEXT: ObjectFileFormat: ELF
+// CHECK-NEXT: Symbols:
+// CHECK-NEXT:   "f" : { Type: Object, Size: 1 }
+// CHECK-NEXT: ...
+auto f = [](void* data) { int i; };