[lld-macho] Fix category merging sed issue - Try nr.2 #112981

alx32 · 2024-10-18T20:55:45Z

We replace sed with awk as I couldn't find a syntax that works consistently on Linux/Mac for sed.
Repro'ed original issue on Mac and confirmed working now on Mac/Linux.

llvmbot · 2024-10-18T20:57:32Z

@llvm/pr-subscribers-lld-macho

@llvm/pr-subscribers-lld

Author: None (alx32)

Changes

We replace sed with awk as I couldn't find a syntax that works consistently on Linux/Mac for sed.
Repro'ed original issue on Mac and confirmed working now on Mac/Linux.

Full diff: https://github.com/llvm/llvm-project/pull/112981.diff

1 Files Affected:

(modified) lld/test/MachO/objc-category-merging-minimal.s (+1-1)

diff --git a/lld/test/MachO/objc-category-merging-minimal.s b/lld/test/MachO/objc-category-merging-minimal.s
index 437294791bf33b..88c175333f2629 100644
--- a/lld/test/MachO/objc-category-merging-minimal.s
+++ b/lld/test/MachO/objc-category-merging-minimal.s
@@ -30,7 +30,7 @@
 
 ############ Test merging skipped due to invalid category name ############
 # Modify __OBJC_$_CATEGORY_MyBaseClass_$_Category01's name to point to L_OBJC_IMAGE_INFO+3
-# RUN: sed -E '/^__OBJC_\$_CATEGORY_MyBaseClass_\$_Category01:/ { n; s/^[ \t]*\.quad[ \t]+l_OBJC_CLASS_NAME_$/\t.quad\tL_OBJC_IMAGE_INFO+3/}' merge_cat_minimal.s > merge_cat_minimal_bad_name.s
+# RUN: awk '/^__OBJC_\$_CATEGORY_MyBaseClass_\$_Category01:/ { print; getline; sub(/^[ \t]*\.quad[ \t]+l_OBJC_CLASS_NAME_$/, "\t.quad\tL_OBJC_IMAGE_INFO+3"); print; next } { print }' merge_cat_minimal.s > merge_cat_minimal_bad_name.s
 
 # Assemble the modified source
 # RUN: llvm-mc -filetype=obj -triple=arm64-apple-macos -o merge_cat_minimal_bad_name.o merge_cat_minimal_bad_name.s

ellishg · 2024-10-18T21:40:50Z

For sed you might want to try using | as the separator.

Example:

llvm-project/bolt/test/X86/register-fragments-bolt-symbols.s

Line 23 in 659192b

# RUN: sed -i 's|chain|chain/2|g' %t.fdata

https://stackoverflow.com/questions/52374114/using-sed-s-act

ellishg · 2024-10-18T21:42:00Z

For sed you might want to try using | as the separator.

Example:

llvm-project/bolt/test/X86/register-fragments-bolt-symbols.s

Line 23 in 659192b

# RUN: sed -i 's|chain|chain/2|g' %t.fdata

https://stackoverflow.com/questions/52374114/using-sed-s-act

I'm not sure if this will fix the issue, though :(

ellishg

Accepting to unblock, but if we can get sed to work I'd prefer that because I think it's simpler.

alx32 · 2024-10-18T22:08:56Z

if we can get sed to work I'd prefer that because I think it's simpler.

Tried these, but still no go:

sed -E '|^__OBJC_\$_CATEGORY_MyBaseClass_\$_Category01:| { n; s|^[ \t]*\.quad[ \t]+l_OBJC_CLASS_NAME_$|\t.quad\tL_OBJC_IMAGE_INFO+3|g }' merge_cat_minimal.s > merge_cat_minimal_bad_name.s

sed -E '/^__OBJC_\$_CATEGORY_MyBaseClass_\$_Category01:/ { n; s|^[ \t]*\.quad[ \t]+l_OBJC_CLASS_NAME_$|\t.quad\tL_OBJC_IMAGE_INFO+3|; }' merge_cat_minimal.s > merge_cat_minimal_bad_name.s

llvm-ci · 2024-10-18T22:54:40Z

LLVM Buildbot has detected a new failure on builder clang-hip-vega20 running on hip-vega20-0 while building lld at step 3 "annotate".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/123/builds/7804

Here is the relevant piece of the build log for the reference

Step 3 (annotate) failure: '../llvm-zorg/zorg/buildbot/builders/annotated/hip-build.sh --jobs=' (failure)
...
[38/40] : && /buildbot/hip-vega20-0/clang-hip-vega20/llvm/bin/clang++ -O3 -DNDEBUG  External/HIP/CMakeFiles/InOneWeekend-hip-6.0.2.dir/workload/ray-tracing/InOneWeekend/main.cc.o -o External/HIP/InOneWeekend-hip-6.0.2  --rocm-path=/buildbot/Externals/hip/rocm-6.0.2 --hip-link -rtlib=compiler-rt -unwindlib=libgcc -frtlib-add-rpath && cd /buildbot/hip-vega20-0/clang-hip-vega20/test-suite-build/External/HIP && /usr/local/bin/cmake -E create_symlink /buildbot/llvm-test-suite/External/HIP/InOneWeekend.reference_output /buildbot/hip-vega20-0/clang-hip-vega20/test-suite-build/External/HIP/InOneWeekend.reference_output-hip-6.0.2
[39/40] /buildbot/hip-vega20-0/clang-hip-vega20/llvm/bin/clang++ -DNDEBUG  -O3 -DNDEBUG   -w -Werror=date-time --rocm-path=/buildbot/Externals/hip/rocm-6.0.2 --offload-arch=gfx908 --offload-arch=gfx90a --offload-arch=gfx1030 --offload-arch=gfx1100 -xhip -mfma -MD -MT External/HIP/CMakeFiles/TheNextWeek-hip-6.0.2.dir/workload/ray-tracing/TheNextWeek/main.cc.o -MF External/HIP/CMakeFiles/TheNextWeek-hip-6.0.2.dir/workload/ray-tracing/TheNextWeek/main.cc.o.d -o External/HIP/CMakeFiles/TheNextWeek-hip-6.0.2.dir/workload/ray-tracing/TheNextWeek/main.cc.o -c /buildbot/llvm-test-suite/External/HIP/workload/ray-tracing/TheNextWeek/main.cc
[40/40] : && /buildbot/hip-vega20-0/clang-hip-vega20/llvm/bin/clang++ -O3 -DNDEBUG  External/HIP/CMakeFiles/TheNextWeek-hip-6.0.2.dir/workload/ray-tracing/TheNextWeek/main.cc.o -o External/HIP/TheNextWeek-hip-6.0.2  --rocm-path=/buildbot/Externals/hip/rocm-6.0.2 --hip-link -rtlib=compiler-rt -unwindlib=libgcc -frtlib-add-rpath && cd /buildbot/hip-vega20-0/clang-hip-vega20/test-suite-build/External/HIP && /usr/local/bin/cmake -E create_symlink /buildbot/llvm-test-suite/External/HIP/TheNextWeek.reference_output /buildbot/hip-vega20-0/clang-hip-vega20/test-suite-build/External/HIP/TheNextWeek.reference_output-hip-6.0.2
+ build_step 'Testing HIP test-suite'
+ echo '@@@BUILD_STEP Testing HIP test-suite@@@'
+ ninja -v check-hip-simple
@@@BUILD_STEP Testing HIP test-suite@@@
[0/1] cd /buildbot/hip-vega20-0/clang-hip-vega20/test-suite-build/External/HIP && /buildbot/hip-vega20-0/clang-hip-vega20/llvm/bin/llvm-lit -sv empty-hip-6.0.2.test with-fopenmp-hip-6.0.2.test saxpy-hip-6.0.2.test memmove-hip-6.0.2.test InOneWeekend-hip-6.0.2.test TheNextWeek-hip-6.0.2.test blender.test
-- Testing: 7 tests, 7 workers --
Testing:  0.. 10.. 20.. 30.. 40
FAIL: test-suite :: External/HIP/TheNextWeek-hip-6.0.2.test (4 of 7)
******************** TEST 'test-suite :: External/HIP/TheNextWeek-hip-6.0.2.test' FAILED ********************

/buildbot/hip-vega20-0/clang-hip-vega20/test-suite-build/tools/timeit-target --timeout 7200 --limit-core 0 --limit-cpu 7200 --limit-file-size 209715200 --limit-rss-size 838860800 --append-exitstatus --redirect-output /buildbot/hip-vega20-0/clang-hip-vega20/test-suite-build/External/HIP/Output/TheNextWeek-hip-6.0.2.test.out --redirect-input /dev/null --summary /buildbot/hip-vega20-0/clang-hip-vega20/test-suite-build/External/HIP/Output/TheNextWeek-hip-6.0.2.test.time /buildbot/hip-vega20-0/clang-hip-vega20/test-suite-build/External/HIP/TheNextWeek-hip-6.0.2
cd /buildbot/hip-vega20-0/clang-hip-vega20/test-suite-build/External/HIP ; /buildbot/hip-vega20-0/clang-hip-vega20/test-suite-build/tools/fpcmp-target /buildbot/hip-vega20-0/clang-hip-vega20/test-suite-build/External/HIP/Output/TheNextWeek-hip-6.0.2.test.out TheNextWeek.reference_output-hip-6.0.2

+ cd /buildbot/hip-vega20-0/clang-hip-vega20/test-suite-build/External/HIP
+ /buildbot/hip-vega20-0/clang-hip-vega20/test-suite-build/tools/fpcmp-target /buildbot/hip-vega20-0/clang-hip-vega20/test-suite-build/External/HIP/Output/TheNextWeek-hip-6.0.2.test.out TheNextWeek.reference_output-hip-6.0.2
/buildbot/hip-vega20-0/clang-hip-vega20/test-suite-build/tools/fpcmp-target: Comparison failed, textual difference between 'M' and 'R'

Input 1:
Memory access fault by GPU node-1 (Agent handle: 0x56460db50170) on address (nil). Reason: Page not present or supervisor privilege.
exit 134

Input 2:
Running quads
image width = 400 height = 400
block size = (16, 16) grid size = (25, 25)
Start rendering by GPU.
Done.
quads_gpu.ppm and quads_ref.ppm are the same.
Running earth
image width = 400 height = 225
block size = (16, 16) grid size = (25, 15)
Start rendering by GPU.
Done.
earth_gpu.ppm and earth_ref.ppm are the same.
Running two_spheres
image width = 400 height = 225
block size = (16, 16) grid size = (25, 15)
Start rendering by GPU.
Done.
two_spheres_gpu.ppm and two_spheres_ref.ppm are the same.
Running two_perlin_spheres
image width = 400 height = 225
block size = (16, 16) grid size = (25, 15)
Start rendering by GPU.
Done.
two_perlin_spheres_gpu.ppm and two_perlin_spheres_ref.ppm are the same.
Step 12 (Testing HIP test-suite) failure: Testing HIP test-suite (failure)
@@@BUILD_STEP Testing HIP test-suite@@@
[0/1] cd /buildbot/hip-vega20-0/clang-hip-vega20/test-suite-build/External/HIP && /buildbot/hip-vega20-0/clang-hip-vega20/llvm/bin/llvm-lit -sv empty-hip-6.0.2.test with-fopenmp-hip-6.0.2.test saxpy-hip-6.0.2.test memmove-hip-6.0.2.test InOneWeekend-hip-6.0.2.test TheNextWeek-hip-6.0.2.test blender.test
-- Testing: 7 tests, 7 workers --
Testing:  0.. 10.. 20.. 30.. 40
FAIL: test-suite :: External/HIP/TheNextWeek-hip-6.0.2.test (4 of 7)
******************** TEST 'test-suite :: External/HIP/TheNextWeek-hip-6.0.2.test' FAILED ********************

/buildbot/hip-vega20-0/clang-hip-vega20/test-suite-build/tools/timeit-target --timeout 7200 --limit-core 0 --limit-cpu 7200 --limit-file-size 209715200 --limit-rss-size 838860800 --append-exitstatus --redirect-output /buildbot/hip-vega20-0/clang-hip-vega20/test-suite-build/External/HIP/Output/TheNextWeek-hip-6.0.2.test.out --redirect-input /dev/null --summary /buildbot/hip-vega20-0/clang-hip-vega20/test-suite-build/External/HIP/Output/TheNextWeek-hip-6.0.2.test.time /buildbot/hip-vega20-0/clang-hip-vega20/test-suite-build/External/HIP/TheNextWeek-hip-6.0.2
cd /buildbot/hip-vega20-0/clang-hip-vega20/test-suite-build/External/HIP ; /buildbot/hip-vega20-0/clang-hip-vega20/test-suite-build/tools/fpcmp-target /buildbot/hip-vega20-0/clang-hip-vega20/test-suite-build/External/HIP/Output/TheNextWeek-hip-6.0.2.test.out TheNextWeek.reference_output-hip-6.0.2

+ cd /buildbot/hip-vega20-0/clang-hip-vega20/test-suite-build/External/HIP
+ /buildbot/hip-vega20-0/clang-hip-vega20/test-suite-build/tools/fpcmp-target /buildbot/hip-vega20-0/clang-hip-vega20/test-suite-build/External/HIP/Output/TheNextWeek-hip-6.0.2.test.out TheNextWeek.reference_output-hip-6.0.2
/buildbot/hip-vega20-0/clang-hip-vega20/test-suite-build/tools/fpcmp-target: Comparison failed, textual difference between 'M' and 'R'

Input 1:
Memory access fault by GPU node-1 (Agent handle: 0x56460db50170) on address (nil). Reason: Page not present or supervisor privilege.
exit 134

Input 2:
Running quads
image width = 400 height = 400
block size = (16, 16) grid size = (25, 25)
Start rendering by GPU.
Done.
quads_gpu.ppm and quads_ref.ppm are the same.
Running earth
image width = 400 height = 225
block size = (16, 16) grid size = (25, 15)
Start rendering by GPU.
Done.
earth_gpu.ppm and earth_ref.ppm are the same.
Running two_spheres
image width = 400 height = 225
block size = (16, 16) grid size = (25, 15)
Start rendering by GPU.
Done.
two_spheres_gpu.ppm and two_spheres_ref.ppm are the same.
Running two_perlin_spheres
image width = 400 height = 225
block size = (16, 16) grid size = (25, 15)
Start rendering by GPU.
Done.
two_perlin_spheres_gpu.ppm and two_perlin_spheres_ref.ppm are the same.
Running simple_light
image width = 400 height = 225
block size = (16, 16) grid size = (25, 15)
Start rendering by GPU.
Done.
simple_light_gpu.ppm and simple_light_ref.ppm are the same.
Running random_spheres

aeubanks · 2024-10-21T17:04:09Z

awk doesn't exist on some Windows platforms, breaking this test on on Windows bots

alx32 · 2024-10-21T17:06:24Z

awk doesn't exist on some Windows platforms, breaking this test on on Windows bots

Is there a way to exclude those specific Windows bots ? I remember the Windows PR build bot succeeded for this.

alx32 · 2024-10-21T18:48:21Z

awk doesn't exist on some Windows platforms, breaking this test on on Windows bots

Could you share a link to a failing bot ?

… on Windows With llvm#112981, the test uses awk, which gnuwin32 doesn't seem to have.

aeubanks · 2024-10-21T19:42:34Z

https://ci.chromium.org/ui/p/chromium/builders/try/win_upload_clang/5846/overview is the failing bot

#113209 to disable the test on Windows. The problem is that LLVM sorta supports building with gnuwin32 tools, although people have moved in the direction of git for windows tools over time. gnuwin32 is very old, but we have sent out patches before to disable tests on Windows due to lack of gnuwin32 support. I will try to upgrade our bots not use gnuwin32, but I'd like to keep things green for now if that's ok

alx32 · 2024-10-21T19:45:14Z

Thanks for the fix!

… on Windows (#113209) With #112981, the test uses awk, which gnuwin32 doesn't seem to have.

[lld-macho] Fix category merging sed issue - Try nr.2

143e5a3

alx32 marked this pull request as ready for review October 18, 2024 20:56

llvmbot added lld lld:MachO labels Oct 18, 2024

alx32 mentioned this pull request Oct 18, 2024

[lld-macho] Improve robustness of ObjC category merging #112618

Merged

alx32 requested review from DataCorrupted and kyulee-com October 18, 2024 20:59

ellishg approved these changes Oct 18, 2024

View reviewed changes

alx32 merged commit b35b583 into llvm:main Oct 18, 2024
11 of 12 checks passed

aeubanks added a commit to aeubanks/llvm-project that referenced this pull request Oct 21, 2024

[lld/Macho][test] Mark objc-category-merging-minimal.s as unsupported…

8430024

… on Windows With llvm#112981, the test uses awk, which gnuwin32 doesn't seem to have.

aeubanks mentioned this pull request Oct 21, 2024

[lld/Macho][test] Mark objc-category-merging-minimal.s as unsupported on Windows #113209

Merged

aeubanks added a commit that referenced this pull request Oct 21, 2024

[lld/Macho][test] Mark objc-category-merging-minimal.s as unsupported…

dca43a1

… on Windows (#113209) With #112981, the test uses awk, which gnuwin32 doesn't seem to have.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[lld-macho] Fix category merging sed issue - Try nr.2 #112981

[lld-macho] Fix category merging sed issue - Try nr.2 #112981

Uh oh!

alx32 commented Oct 18, 2024 •

edited

Loading

Uh oh!

llvmbot commented Oct 18, 2024 •

edited

Loading

Uh oh!

ellishg commented Oct 18, 2024

Uh oh!

ellishg commented Oct 18, 2024

Uh oh!

ellishg left a comment

Uh oh!

alx32 commented Oct 18, 2024 •

edited

Loading

Uh oh!

Uh oh!

llvm-ci commented Oct 18, 2024

Uh oh!

aeubanks commented Oct 21, 2024

Uh oh!

alx32 commented Oct 21, 2024

Uh oh!

alx32 commented Oct 21, 2024

Uh oh!

aeubanks commented Oct 21, 2024

Uh oh!

alx32 commented Oct 21, 2024

Uh oh!

Uh oh!

[lld-macho] Fix category merging sed issue - Try nr.2 #112981

[lld-macho] Fix category merging sed issue - Try nr.2 #112981

Uh oh!

Conversation

alx32 commented Oct 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Oct 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ellishg commented Oct 18, 2024

Uh oh!

ellishg commented Oct 18, 2024

Uh oh!

ellishg left a comment

Choose a reason for hiding this comment

Uh oh!

alx32 commented Oct 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

llvm-ci commented Oct 18, 2024

Uh oh!

aeubanks commented Oct 21, 2024

Uh oh!

alx32 commented Oct 21, 2024

Uh oh!

alx32 commented Oct 21, 2024

Uh oh!

aeubanks commented Oct 21, 2024

Uh oh!

alx32 commented Oct 21, 2024

Uh oh!

Uh oh!

alx32 commented Oct 18, 2024 •

edited

Loading

llvmbot commented Oct 18, 2024 •

edited

Loading

alx32 commented Oct 18, 2024 •

edited

Loading