Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test failures rolling into the sdk repo due to recent commits #130

Closed
devoncarew opened this issue Sep 12, 2023 · 7 comments · Fixed by #132
Closed

test failures rolling into the sdk repo due to recent commits #130

devoncarew opened this issue Sep 12, 2023 · 7 comments · Fixed by #132

Comments

@devoncarew
Copy link
Member

We're seeing test failures rolling this repo into the sdk; here's an example of the failures:

https://logs.chromium.org/logs/dart/buildbucket/cr-buildbucket/8770140990951165617/+/u/test_results/new_test_failures__logs_

/============================================================================================\
| native_toolchain_c/test/cbuilder/cbuilder_test broke (Pass -> RuntimeError, expected Pass) |
\============================================================================================/

...

00:02 �[32m+8�[0m�[31m -1�[0m: CBuilder executable (debug, no_pic) �[1m�[31m[E]�[0m�[0m

  ProcessException: Full command string: '/b/s/w/ir/cache/builder/sdk/buildtools/linux-x64/clang/bin/clang /b/s/w/ir/cache/builder/sdk/third_party/pkg/native/pkgs/native_toolchain_c/test/cbuilder/testfiles/hello_world/src/hello_world.c -o /b/s/w/ir/x/t/QXHGIN/hello_world -fno-PIC -fno-PIE -DDEBUG'.
  Exit code: '1'.
  For the output of the process check the logger output.
    Command: /b/s/w/ir/cache/builder/sdk/buildtools/linux-x64/clang/bin/clang /b/s/w/ir/cache/builder/sdk/third_party/pkg/native/pkgs/native_toolchain_c/test/cbuilder/testfiles/hello_world/src/hello_world.c -o /b/s/w/ir/x/t/QXHGIN/hello_world -fno-PIC -fno-PIE -DDEBUG

  package:native_toolchain_c/src/utils/run_process.dart 92:5                             runProcess
  ===== asynchronous gap ===========================
  package:native_toolchain_c/src/cbuilder/run_cbuilder.dart 107:5                        RunCBuilder.runClangLike
  ===== asynchronous gap ===========================
  package:native_toolchain_c/src/cbuilder/run_cbuilder.dart 88:7                         RunCBuilder.run
  ===== asynchronous gap ===========================
  package:native_toolchain_c/src/cbuilder/cbuilder.dart 170:7                            CBuilder.run
  ===== asynchronous gap ===========================
  third_party/pkg/native/pkgs/native_toolchain_c/test/cbuilder/cbuilder_test.dart 61:11  main.<fn>.<fn>
  ===== asynchronous gap ===========================
  third_party/pkg/native/pkgs/native_toolchain_c/test/helpers.dart 52:5                  inTempDir
  ===== asynchronous gap ===========================
  third_party/pkg/native/pkgs/native_toolchain_c/test/cbuilder/cbuilder_test.dart 30:9   main.<fn>
  ===== asynchronous gap ===========================
  package:test_api/src/backend/declarer.dart 215:9                                       Declarer.test.<fn>.<fn>
  ===== asynchronous gap ===========================
  package:test_api/src/backend/declarer.dart 213:7                                       Declarer.test.<fn>
  ===== asynchronous gap ===========================
  package:test_api/src/backend/invoker.dart 258:9                                        Invoker._waitForOutstandingCallbacks.<fn>
  

FINER: 2023-09-12 10:40:42.249655: Using compiler /b/s/w/ir/cache/builder/sdk/buildtools/linux-x64/clang/bin/clang from config[c_compiler.cc].

FINER: 2023-09-12 10:40:42.249923: Trying to recognize file:///b/s/w/ir/cache/builder/sdk/buildtools/linux-x64/clang/bin/clang.

INFO: 2023-09-12 10:40:42.250073: Running `/b/s/w/ir/cache/builder/sdk/buildtools/linux-x64/clang/bin/clang --version`.

FINE: 2023-09-12 10:40:42.288416: Fuchsia clang version 17.0.0 (https://llvm.googlesource.com/llvm-project 6d667d4b261e81f325756fdfd5bb43b3b3d2451d)

FINE: 2023-09-12 10:40:42.288901: Target: x86_64-unknown-linux-gnu

FINE: 2023-09-12 10:40:42.289012: Thread model: posix

FINE: 2023-09-12 10:40:42.289092: InstalledDir: /b/s/w/ir/cache/builder/sdk/buildtools/linux-x64/clang/bin

FINE: 2023-09-12 10:40:42.294722: Tool instance file:///b/s/w/ir/cache/builder/sdk/buildtools/linux-x64/clang/bin/clang is likely Tool(Clang).

FINER: 2023-09-12 10:40:42.294944: Looking up version with --version for ToolInstance(Clang, null, file:///b/s/w/ir/cache/builder/sdk/buildtools/linux-x64/clang/bin/clang).

INFO: 2023-09-12 10:40:42.295225: Running `/b/s/w/ir/cache/builder/sdk/buildtools/linux-x64/clang/bin/clang --version`.

FINE: 2023-09-12 10:40:42.333616: Fuchsia clang version 17.0.0 (https://llvm.googlesource.com/llvm-project 6d667d4b261e81f325756fdfd5bb43b3b3d2451d)

FINE: 2023-09-12 10:40:42.334099: Target: x86_64-unknown-linux-gnu

FINE: 2023-09-12 10:40:42.334193: Thread model: posix

FINE: 2023-09-12 10:40:42.334266: InstalledDir: /b/s/w/ir/cache/builder/sdk/buildtools/linux-x64/clang/bin

FINE: 2023-09-12 10:40:42.339405: Found version for ToolInstance(Clang, 17.0.0, file:///b/s/w/ir/cache/builder/sdk/buildtools/linux-x64/clang/bin/clang).

INFO: 2023-09-12 10:40:42.340326: Running `/b/s/w/ir/cache/builder/sdk/buildtools/linux-x64/clang/bin/clang /b/s/w/ir/cache/builder/sdk/third_party/pkg/native/pkgs/native_toolchain_c/test/cbuilder/testfiles/hello_world/src/hello_world.c -o /b/s/w/ir/x/t/QXHGIN/hello_world -fno-PIC -fno-PIE -DDEBUG`.

SEVERE: 2023-09-12 10:40:42.480816: ld.lld: error: relocation R_X86_64_64 cannot be used against local symbol; recompile with -fPIC

SEVERE: 2023-09-12 10:40:42.481177: >>> defined in /b/s/w/ir/x/t/hello_world-cce0e5.o

SEVERE: 2023-09-12 10:40:42.481288: >>> referenced by hello_world.c

SEVERE: 2023-09-12 10:40:42.481383: >>>               /b/s/w/ir/x/t/hello_world-cce0e5.o:(main)

SEVERE: 2023-09-12 10:40:42.481458:

SEVERE: 2023-09-12 10:40:42.481531: ld.lld: error: relocation R_X86_64_64 cannot be used against local symbol; recompile with -fPIC

SEVERE: 2023-09-12 10:40:42.481630: >>> defined in /b/s/w/ir/x/t/hello_world-cce0e5.o

SEVERE: 2023-09-12 10:40:42.481729: >>> referenced by hello_world.c

SEVERE: 2023-09-12 10:40:42.481803: >>>               /b/s/w/ir/x/t/hello_world-cce0e5.o:(main)

SEVERE: 2023-09-12 10:40:42.489991: clang: error: linker command failed with exit code 1 (use -v to see invocation)

00:02 �[32m+8�[0m�[31m -1�[0m: CBuilder executable (release, no_pic)�[0m

00:02 �[32m+8�[0m�[31m -1�[0m: CBuilder executable (debug, no_pic)�[0m


FINER: 2023-09-12 10:40:42.547201: Using compiler /b/s/w/ir/cache/builder/sdk/buildtools/linux-x64/clang/bin/clang from config[c_compiler.cc].


FINER: 2023-09-12 10:40:42.547622: Trying to recognize file:///b/s/w/ir/cache/builder/sdk/buildtools/linux-x64/clang/bin/clang.


INFO: 2023-09-12 10:40:42.547928: Running `/b/s/w/ir/cache/builder/sdk/buildtools/linux-x64/clang/bin/clang --version`.


FINE: 2023-09-12 10:40:42.587447: Fuchsia clang version 17.0.0 (https://llvm.googlesource.com/llvm-project 6d667d4b261e81f325756fdfd5bb43b3b3d2451d)


FINE: 2023-09-12 10:40:42.588218: Target: x86_64-unknown-linux-gnu


FINE: 2023-09-12 10:40:42.588362: Thread model: posix


FINE: 2023-09-12 10:40:42.588533: InstalledDir: /b/s/w/ir/cache/builder/sdk/buildtools/linux-x64/clang/bin


FINE: 2023-09-12 10:40:42.589710: Tool instance file:///b/s/w/ir/cache/builder/sdk/buildtools/linux-x64/clang/bin/clang is likely Tool(Clang).


FINER: 2023-09-12 10:40:42.590048: Looking up version with --version for ToolInstance(Clang, null, file:///b/s/w/ir/cache/builder/sdk/buildtools/linux-x64/clang/bin/clang).


INFO: 2023-09-12 10:40:42.590416: Running `/b/s/w/ir/cache/builder/sdk/buildtools/linux-x64/clang/bin/clang --version`.


FINE: 2023-09-12 10:40:42.630397: Fuchsia clang version 17.0.0 (https://llvm.googlesource.com/llvm-project 6d667d4b261e81f325756fdfd5bb43b3b3d2451d)


FINE: 2023-09-12 10:40:42.631266: Target: x86_64-unknown-linux-gnu


FINE: 2023-09-12 10:40:42.631437: Thread model: posix


FINE: 2023-09-12 10:40:42.631561: InstalledDir: /b/s/w/ir/cache/builder/sdk/buildtools/linux-x64/clang/bin


FINE: 2023-09-12 10:40:42.634174: Found version for ToolInstance(Clang, 17.0.0, file:///b/s/w/ir/cache/builder/sdk/buildtools/linux-x64/clang/bin/clang).


INFO: 2023-09-12 10:40:42.635168: Running `/b/s/w/ir/cache/builder/sdk/buildtools/linux-x64/clang/bin/clang /b/s/w/ir/cache/builder/sdk/third_party/pkg/native/pkgs/native_toolchain_c/test/cbuilder/testfiles/hello_world/src/hello_world.c -o /b/s/w/ir/x/t/TQVLCL/hello_world -fno-PIC -fno-PIE -DRELEASE -DNDEBUG`.


SEVERE: 2023-09-12 10:40:42.736286: ld.lld: error: relocation R_X86_64_64 cannot be used against local symbol; recompile with -fPIC


SEVERE: 2023-09-12 10:40:42.737074: >>> defined in /b/s/w/ir/x/t/hello_world-2a68b7.o


SEVERE: 2023-09-12 10:40:42.737256: >>> referenced by hello_world.c


SEVERE: 2023-09-12 10:40:42.737416: >>>               /b/s/w/ir/x/t/hello_world-2a68b7.o:(main)


SEVERE: 2023-09-12 10:40:42.744498: clang: error: linker command failed with exit code 1 (use -v to see invocation)
@devoncarew
Copy link
Member Author

The last successful rev was from a2dfedc.

@dcharkes
Copy link
Collaborator

SEVERE: 2023-09-12 10:40:42.736286: ld.lld: error: relocation R_X86_64_64 cannot be used against local symbol; recompile with -fPIC
SEVERE: 2023-09-12 10:40:42.737074: >>> defined in /b/s/w/ir/x/t/hello_world-2a68b7.o
SEVERE: 2023-09-12 10:40:42.737256: >>> referenced by hello_world.c

@blaugold this might be a result of your PR.

I'll dig into what the error means tomorrow.

@blaugold
Copy link
Contributor

Before 387f894 we let the compiler decide whether to generate PIC. After that change, we are strict about not generating PIC when pic is set to false. I guess what we did not catch in the GitHub Actions CI is compiling for Linux with clang?

@dcharkes
Copy link
Collaborator

Based on https://groups.google.com/g/llvm-dev/c/bcmpM2ALmJs, I think we might want to add -z notext which enables relocations in readonly sections. https://reviews.llvm.org/D30530

Applying that locally in the Dart SDK checkout on the latest version of the package makes the tests pass.

diff --git a/pkgs/native_toolchain_c/lib/src/cbuilder/run_cbuilder.dart b/pkgs/native_toolchain_c/lib/src/cbuilder/run_cbuilder.dart
index 89d0f3e..2a44526 100644
--- a/pkgs/native_toolchain_c/lib/src/cbuilder/run_cbuilder.dart
+++ b/pkgs/native_toolchain_c/lib/src/cbuilder/run_cbuilder.dart
@@ -160,6 +160,8 @@ class RunCBuilder {
           ] else ...[
             '-fno-PIC',
             '-fno-PIE',
+            '-z',
+            'notext',
           ],
         for (final MapEntry(key: name, :value) in defines.entries)
           if (value == null) '-D$name' else '-D$name=$value'

We should check if all compilers accept -z notext.

@blaugold Do you have a Dart SDK checkout?

In a Dart SDK checkout on Linux the tests can be run with the clang pinned in the Dart SDK with:

~/dart-sdk/sdk$ tools/test.py --build -n unittest-asserts-release-linux native_toolchain_c/test

The way to test changes on this repo with the Dart SDK are:

  1. Go to the DEPS file and update the rev_native hash with the hash you want to test. Note that the mirror of this repo is sometimes delayed. If you want to test a branch from your GitHub mirror, you also need to update the line with Var("dart_git") + "native.git" + "@" + Var("native_rev"), to point to your GitHub.
  2. Run gclient sync -f which pulls in the sources to third_party/pkg/native
  3. Optional: modify the sources in third_party/pkg/native
  4. Run $ tools/test.py --build -n unittest-asserts-release-linux native_toolchain_c/test

@dcharkes
Copy link
Collaborator

@blaugold We pin the compiler in the Dart SDK (usually a very recent version from the clang main branch), and we rely on the compiler that the GitHub CI provides for us on the GitHub CI. These can be different. That's why we didn't catch this on the GitHub CI.

I've filed #131 to investigate if we can get a roller-job on the CI here.

@dcharkes
Copy link
Collaborator

Keeping this open until the roll goes through in the SDK: https://dart-review.googlesource.com/c/sdk/+/326020

@dcharkes dcharkes reopened this Sep 14, 2023
@blaugold
Copy link
Contributor

@dcharkes Sorry, I didn't have time yesterday to work on a fix. Thanks for taking care of it!

copybara-service bot pushed a commit to dart-lang/sdk that referenced this issue Sep 14, 2023
Bug: dart-lang/native#130
Change-Id: I6ec9f934c227b7eb9f8ee724598d9d557e42dab6
Cq-Include-Trybots: luci.dart.try:pkg-linux-debug-try,pkg-linux-release-try,pkg-mac-release-arm64-try,pkg-mac-release-try,pkg-win-release-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/326020
Commit-Queue: Daco Harkes <dacoharkes@google.com>
Reviewed-by: Hossein Yousefi <yousefi@google.com>
HosseinYousefi added a commit that referenced this issue Nov 16, 2023
HosseinYousefi added a commit that referenced this issue Nov 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants