fix build_id logic for fuchsia symbols #16397

dnfield · 2020-02-05T18:38:37Z

llvm-readelf --hex-dump=.note.gnu.build-id gives output like this:

Hex dump of section '.note.gnu.build-id':
0x000002e8 04000000 14000000 03000000 474e5500 ............GNU.
0x000002f8 e2911dbc 6820ea37 22d5bafa 9fee3db3 ....h .7".....=.
0x00000308 d6fcf810                            ....

We're then taking the last line (into a var named second_line!), including the string representation of the hex bytes, which sometimes results in getting a /. This causes failures like https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket.appspot.com/8889296931334102496/+/steps/build_fuchsia_debug_x64_flutter_shell_platform_fuchsia:fuchsia_fuchsia_tests/0/stdout, because you can't have a file with a / in it on a POSIX file system.

This also isn't actually getting the right build id. This patch uses llvm-readelf -n to get the nice buildID we want:

Displaying notes found at file offset 0x000002e8 with length 0x00000024:
  Owner                Data size 	Description
  GNU                  0x00000014	NT_GNU_BUILD_ID (unique build ID bitstring)
    Build ID: 0a1ca5114c5c5548ee483781585f5e89228f9e66

I then still tried to keep the old logic, but I'm not 100% sure about the directory structure. This will result in a file like:

out/fuchsia_debug_x64/flutter-debug-symbols-debug-fuchsia-x64/e2/911dbc6820ea3722d5bafa9fee3db3d6fcf810.debug, which I'm assuming is correct for a buildID of e2911dbc6820ea3722d5bafa9fee3db3d6fcf810.

I've also changed the retry logic so that it is just ok if the directory exists, and only fails if we failed to create the directory. It fails sometimes because another process is also creating the directoyr we're trying to create and just got there first. This avoids the loop and the extraneous error messages during build.

dnfield · 2020-02-05T18:41:11Z

Assuming this is approved, thsi should be landed on red - the current cause of redness in the tree is the failure in the existing logic.

iskakaushik

LGTM

chinmaygarde · 2020-02-05T18:42:20Z

tools/fuchsia/copy_debug_symbols.py

-  second_line = file_out.splitlines()[-1].split()
-  build_id = second_line[1] + second_line[2]
+  file_out = subprocess.check_output([read_elf, '-n', exec_path])
+  build_id = file_out.splitlines()[-1].split()[-1]


Can we look for a hex string instead? This depends on the output of readelf being stable.

I'd also vote for more error message here. Something like, "Tried to find a hex string of the SHA1 hash of the text sections in the ELF file as the build_id. But failed because of...".

Yeah that would be better.

Is it safe to assume it will be a SHA-1 hex string? That way we can assert on the length and content.

If we change it, I'd rather it fail right away. Also, please log that we are explicitly looking for a SHA1 hex string in the build-id when you assert. Asserting here would definitely be better than failing because we failed to create some directory later. This error manifested way to far away from where the issue was occurred.

Updated - PTAL

chinmaygarde · 2020-02-05T20:56:02Z

Can we land this? This should unblock LUCI.

dnfield · 2020-02-05T20:57:06Z

I'm still seeing crashes, trying to make sure it's not anything in this...

dnfield · 2020-02-05T20:59:47Z

Ah. I'm going to land this.

There's a bug in the recipes for try jobs.

dnfield · 2020-02-05T21:02:35Z

Filed flutter/flutter#50211 to fix the other issue seen in this PR.

* 9347c93 Roll src/third_party/dart c8ed304e979a..3414b5167554 (52 commits) (flutter/engine#16362) * 16cd6f0 Roll fuchsia/sdk/core/mac-amd64 from 6h3IH... to Ke00Y... (flutter/engine#16360) * 8c6cc65 Fix runtime type errors when running with canvaskit (flutter/engine#16312) * 677b563 Refactor of Vulkan GPUSurface code (flutter/engine#16224) * 7ca44d3 Kill the test harness if any test exceeds a timeout. (flutter/engine#16349) * 7f6149c Revert "Remove use of the deprecated AccessibilityNodeInfo boundsInPa… (flutter/engine#16355) * 488f489 Roll fuchsia/sdk/core/linux-amd64 from Tszo5... to VJv0H... (flutter/engine#16363) * 7c9b11a Roll src/third_party/skia 71ce449d2814..2aee7d24da8f (5 commits) (flutter/engine#16364) * 7e1d144 Expose enable-service-port-fallback switch (flutter/engine#16366) * 4cda916 Expose the dart kernel snapshot target and copied assets as a public dependency (flutter/engine#16266) * 73c5130 Roll src/third_party/dart 3414b5167554..68e904e444dc (17 commits) (flutter/engine#16367) * 1cd8f3b Fix and consolidate wstring conversion utils (flutter/engine#16342) * b98a39e Add docs (flutter/engine#16368) * e3e6de2 Roll buildroot to c44791c89d2b51bfce864ab2cf5228d41ece1fcc (flutter/engine#16371) * e24ec59 Fuchsia a11y actions (flutter/engine#16321) * d8400c9 Roll src/third_party/skia 2aee7d24da8f..14d64afaa8a3 (10 commits) (flutter/engine#16374) * eeabd4d Roll src/third_party/dart 68e904e444dc..48808f7dce81 (17 commits) (flutter/engine#16377) * 22b994c Roll fuchsia/sdk/core/mac-amd64 from Ke00Y... to ubThi... (flutter/engine#16378) * 0471f44 Roll src/third_party/skia 14d64afaa8a3..6c9b1fd6663e (7 commits) (flutter/engine#16380) * 852d824 Roll src/third_party/dart 48808f7dce81..34893faa6079 (7 commits) (flutter/engine#16381) * 4aa2083 Roll src/third_party/skia 6c9b1fd6663e..bc3307c395e2 (1 commits) (flutter/engine#16383) * 036c370 Copied Apple's semantics for switches, made checkboxes the same. (flutter/engine#16211) * c107969 fix build_id logic for fuchsia symbols (flutter/engine#16397) * 11b7704 [fuchsia] Migrate flutter runner to use Present2 (flutter/engine#14162) * 646ec35 Update license output (flutter/engine#16379) * 925c60b Fix Windows embedding issues caught by clang (flutter/engine#16369) * 31bf3e1 Roll src/third_party/dart 34893faa6079..b3396cbdcae1 (36 commits) (flutter/engine#16422) * 8f89bac [web] Fixes incorrect transform when context save and transforms are deferred. (flutter/engine#16412) * f34bc65 use percent for golden diff rates; tighten the values (flutter/engine#16430)

This reverts commit 2a0ae51.

fix build_id logic for fuchsia symbols

b060f85

dnfield requested review from chinmaygarde and iskakaushik February 5, 2020 18:38

googlebot added the cla: yes label Feb 5, 2020

auto-assign bot requested a review from cbracken February 5, 2020 18:38

iskakaushik approved these changes Feb 5, 2020

View reviewed changes

chinmaygarde reviewed Feb 5, 2020

View reviewed changes

chinmaygarde mentioned this pull request Feb 5, 2020

Add a unit-test harness for scripts used by infra. flutter/flutter#50207

Closed

dnfield added 2 commits February 5, 2020 11:03

..

360742c

space

d285699

chinmaygarde approved these changes Feb 5, 2020

View reviewed changes

dnfield added 2 commits February 5, 2020 11:20

loosen build ID length restrictions

e5c19aa

length greater than 2

b3f8eeb

dnfield force-pushed the fix_buildid branch from c48775b to b3f8eeb Compare February 5, 2020 19:22

dnfield added 5 commits February 5, 2020 11:24

a-f

9e16124

handle dash

cc79627

typo

83081b8

unused var...

ca00d10

Missing import.. it works locally this time, I promise.

d68c677

dnfield merged commit c107969 into flutter:master Feb 5, 2020

dnfield deleted the fix_buildid branch February 5, 2020 20:59

engine-flutter-autoroll mentioned this pull request Feb 5, 2020

Roll engine 81dffd1241e6..c107969ecd4e (23 commits) flutter/flutter#50217

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 5, 2020

c107969 fix build_id logic for fuchsia symbols (flutter/engine#16397)

7712ad0

engine-flutter-autoroll mentioned this pull request Feb 5, 2020

Roll engine 81dffd1241e6..11b770489f1e (24 commits) flutter/flutter#50227

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 5, 2020

c107969 fix build_id logic for fuchsia symbols (flutter/engine#16397)

bcf81bb

engine-flutter-autoroll mentioned this pull request Feb 6, 2020

Roll engine 81dffd1241e6..925c60b38bad (26 commits) flutter/flutter#50238

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 6, 2020

c107969 fix build_id logic for fuchsia symbols (flutter/engine#16397)

de96623

engine-flutter-autoroll mentioned this pull request Feb 6, 2020

Roll engine 81dffd1241e6..8f89bac4b785 (28 commits) flutter/flutter#50250

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 6, 2020

c107969 fix build_id logic for fuchsia symbols (flutter/engine#16397)

0907479

engine-flutter-autoroll mentioned this pull request Feb 6, 2020

Roll engine 81dffd1241e6..f34bc65bee49 (29 commits) flutter/flutter#50255

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 6, 2020

c107969 fix build_id logic for fuchsia symbols (flutter/engine#16397)

644cc1c

NoamDev pushed a commit to NoamDev/engine that referenced this pull request Feb 27, 2020

fix build_id logic for fuchsia symbols (flutter#16397)

2a0ae51

NoamDev added a commit to NoamDev/engine that referenced this pull request Feb 27, 2020

Revert "fix build_id logic for fuchsia symbols (flutter#16397)"

ef6774c

This reverts commit 2a0ae51.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix build_id logic for fuchsia symbols #16397

fix build_id logic for fuchsia symbols #16397

dnfield commented Feb 5, 2020

dnfield commented Feb 5, 2020

iskakaushik left a comment

chinmaygarde Feb 5, 2020

chinmaygarde Feb 5, 2020

dnfield Feb 5, 2020

chinmaygarde Feb 5, 2020

dnfield Feb 5, 2020

chinmaygarde commented Feb 5, 2020

dnfield commented Feb 5, 2020

dnfield commented Feb 5, 2020

dnfield commented Feb 5, 2020

fix build_id logic for fuchsia symbols #16397

fix build_id logic for fuchsia symbols #16397

Conversation

dnfield commented Feb 5, 2020

dnfield commented Feb 5, 2020

iskakaushik left a comment

Choose a reason for hiding this comment

chinmaygarde Feb 5, 2020

Choose a reason for hiding this comment

chinmaygarde Feb 5, 2020

Choose a reason for hiding this comment

dnfield Feb 5, 2020

Choose a reason for hiding this comment

chinmaygarde Feb 5, 2020

Choose a reason for hiding this comment

dnfield Feb 5, 2020

Choose a reason for hiding this comment

chinmaygarde commented Feb 5, 2020

dnfield commented Feb 5, 2020

dnfield commented Feb 5, 2020

dnfield commented Feb 5, 2020