[PATCH] D105210: [lld-macho] Ignore debug symbols for now.

David Blaikie via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jul 2 15:44:43 PDT 2021


dblaikie added inline comments.


================
Comment at: lld/test/MachO/bug_50812.s:6
+# RUN: yaml2obj %s -o %t.o
+# RUN: %lld -lSystem -platform_version macos 11.3 11.0 -arch x86_64 %t.o
+--- !mach-o
----------------
oontvoo wrote:
> dblaikie wrote:
> > I fixed an issue with this test in bf7f846b683077a8adcb229624082e525870229b by adding `-o %t` to the lld RUN line - without that it tries to write the linked executable to the current working directory which may not be writable/may not be suitable to write to. (might be the test source directory, for instance)
> > 
> > But also: This test should probably verify something about the output of the lld execution - currently this test checks for "does anything so long as it exits 0" - perhaps it should check the specific behavior that was untested previously? What specific features of the resulting binary were untested previously/hidden behind this incorrect error?
> > I fixed an issue with this test in bf7f846b683077a8adcb229624082e525870229b by adding `-o %t` to the lld RUN line - without that it tries to write the linked executable to the current working directory which may not be writable/may not be suitable to write to. (might be the test source directory, for instance)
> 
> Thanks! (Yeah, I forgot this had caused test failures before when someone was doing an integrate)
> 
>  
> > But also: This test should probably verify something about the output of the lld execution - currently this test checks for "does anything so long as it exits 0" - perhaps it should check the specific behavior that was untested previously? What specific features of the resulting binary were untested previously/hidden behind this incorrect error?
> 
> This is not a complete binary (because the yaml was trimmed down to make the test small) so the resulting binary wouldn't have run.
> The specific bug we were testing was the fact that debug symbols being present in .o files, which wasn't anticipated before. 
> 
> This is not a complete binary (because the yaml was trimmed down to make the test small) so the resulting binary wouldn't have run.

Tests don't run linked binaries anyway - they might not be built for the host target, etc.

I meant testing some properties of the linked binary - same as other lld tests do. (objdumping them to check certain features are present, etc)

> The specific bug we were testing was the fact that debug symbols being present in .o files, which wasn't anticipated before.

So it sounds like maybe this test should be checking that the resulting binary doesn't have any of these debug symbols present?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105210/new/

https://reviews.llvm.org/D105210



More information about the llvm-commits mailing list