[llvm] a17c90d - Re-land "[dsymutil] Account for DW_OP_convert being CU relative"

Sun Jul 17 16:11:54 PDT 2022

When recommitting it'd be handy to have a summary, in the commit
message, of how the reason for revert was addressed (if it involved
changes to this patch, what were they, if it involved preliminary
commits to prepare the codebase for the change, what were those, etc)

On Tue, Jul 12, 2022 at 6:47 PM Jonas Devlieghere via llvm-commits
<llvm-commits at lists.llvm.org> wrote:
>
>
> Author: Jonas Devlieghere
> Date: 2022-07-12T18:46:23-07:00
> New Revision: a17c90daf2e7c3b1817ec29ad6648ce89b927f9a
>
> URL: https://github.com/llvm/llvm-project/commit/a17c90daf2e7c3b1817ec29ad6648ce89b927f9a
> DIFF: https://github.com/llvm/llvm-project/commit/a17c90daf2e7c3b1817ec29ad6648ce89b927f9a.diff
>
> LOG: Re-land "[dsymutil] Account for DW_OP_convert being CU relative"
>
> Currently, dsymutil treats the DW_OP_convert operand as absolute instead
> of CU relative, as described by in the DWARFv5 spec, 2.5.1.6:
>
>   "[DW_OP_convert] takes one operand, which is an unsigned LEB128 integer
>   that represents the offset of a debugging information entry in the current
>   compilation unit"
>
> This patch makes dsymutil correctly treat the offset as CU relative,
> preventing a crash when there are multiple compilation units.
>
> Big thanks to Akira Hatanaka for figuring out this issue and providing
> both a reduced test case and a proposed fix.
>
> Added:
>     llvm/test/tools/dsymutil/Inputs/private/tmp/op-convert-offset/op-convert-offset
>     llvm/test/tools/dsymutil/Inputs/private/tmp/op-convert-offset/op-convert-offset.o
>     llvm/test/tools/dsymutil/X86/op-convert-offset.test
>
> Modified:
>     llvm/lib/DWARFLinker/DWARFLinker.cpp
>
> Removed:
>
>
>
> ################################################################################
> diff  --git a/llvm/lib/DWARFLinker/DWARFLinker.cpp b/llvm/lib/DWARFLinker/DWARFLinker.cpp
> index 50c52190c1f61..298359dea9af5 100644
> --- a/llvm/lib/DWARFLinker/DWARFLinker.cpp
> +++ b/llvm/lib/DWARFLinker/DWARFLinker.cpp
> @@ -1005,6 +1005,7 @@ void DWARFLinker::DIECloner::cloneExpression(
>        // instead indicate the generic type. The same holds for
>        // DW_OP_reinterpret, which is currently not supported.
>        if (RefOffset > 0 || Op.getCode() != dwarf::DW_OP_convert) {
> +        RefOffset += Unit.getOrigUnit().getOffset();
>          auto RefDie = Unit.getOrigUnit().getDIEForOffset(RefOffset);
>          CompileUnit::DIEInfo &Info = Unit.getInfo(RefDie);
>          if (DIE *Clone = Info.Clone)
>
> diff  --git a/llvm/test/tools/dsymutil/Inputs/private/tmp/op-convert-offset/op-convert-offset b/llvm/test/tools/dsymutil/Inputs/private/tmp/op-convert-offset/op-convert-offset
> new file mode 100755
> index 0000000000000..d9cf1dac928ac
> Binary files /dev/null and b/llvm/test/tools/dsymutil/Inputs/private/tmp/op-convert-offset/op-convert-offset
> diff er
>
> diff  --git a/llvm/test/tools/dsymutil/Inputs/private/tmp/op-convert-offset/op-convert-offset.o b/llvm/test/tools/dsymutil/Inputs/private/tmp/op-convert-offset/op-convert-offset.o
> new file mode 100644
> index 0000000000000..087e4e7f37c35
> Binary files /dev/null and b/llvm/test/tools/dsymutil/Inputs/private/tmp/op-convert-offset/op-convert-offset.o
> diff er
>
> diff  --git a/llvm/test/tools/dsymutil/X86/op-convert-offset.test b/llvm/test/tools/dsymutil/X86/op-convert-offset.test
> new file mode 100644
> index 0000000000000..e881afa10c0dd
> --- /dev/null
> +++ b/llvm/test/tools/dsymutil/X86/op-convert-offset.test
> @@ -0,0 +1,45 @@
> +# To rebuild the test case:
> +#
> +# $ cat op-convert-offset0.c
> +# extern int a[2];
> +#
> +# int foo(_Bool b) {
> +#   return b ? a[1] : a[0];
> +# }
> +# $ cat op-convert-offset1.c
> +# int a[2] = {1, 2};
> +#
> +# int foo(_Bool);
> +#
> +# int main() {
> +#   return foo(0);
> +# }
> +# $ xcrun clang -g -O2 -c -target x86_64-apple-unknown-macho op-convert-offset1.c -emit-llvm
> +# $ xcrun clang -g -O2 -c -target x86_64-apple-unknown-macho op-convert-offset0.c -emit-llvm
> +# $ llvm-link op-convert-offset1.bc op-convert-offset0.bc -o op-convert-offset.ll -S
> +# $ xcrun clang -c op-convert-offset.ll -O0 -arch x86_64
> +# $ xcrun clang -g op-convert-offset.o -O0 -arch x86_64 -o op-convert-offset
> +
> +RUN: dsymutil -oso-prepend-path %p/../Inputs %p/../Inputs/private/tmp/op-convert-offset/op-convert-offset -o %t.dSYM 2>&1
> +RUN: llvm-dwarfdump %p/../Inputs/private/tmp/op-convert-offset/op-convert-offset.o 2>&1 | FileCheck %s --check-prefix OBJ
> +RUN: llvm-dwarfdump %t.dSYM 2>&1 | FileCheck %s --check-prefix DSYM
> +
> +OBJ: 0x0000007d:   DW_TAG_base_type
> +OBJ:                 DW_AT_name      ("DW_ATE_unsigned_1")
> +OBJ:                 DW_AT_encoding  (DW_ATE_unsigned)
> +OBJ:                 DW_AT_byte_size (0x01)
> +
> +OBJ: 0x00000094:     DW_TAG_formal_parameter
> +OBJ:                   DW_AT_location        (DW_OP_breg2 RCX+0, DW_OP_constu 0xff, DW_OP_and, DW_OP_convert (0x0000007d) "DW_ATE_unsigned_1", DW_OP_convert (0x00000081) "DW_ATE_unsigned_8", DW_OP_stack_value)
> +OBJ:                   DW_AT_name    ("b")
> +OBJ:                   DW_AT_type    (0x000000af "_Bool")
> +
> +DSYM: 0x00000095:   DW_TAG_base_type
> +DSYM:                 DW_AT_name      ("DW_ATE_unsigned_1")
> +DSYM:                 DW_AT_encoding  (DW_ATE_unsigned)
> +DSYM:                 DW_AT_byte_size (0x01)
> +
> +DSYM: 0x000000bc:     DW_TAG_formal_parameter
> +DSYM:                   DW_AT_location        (DW_OP_breg2 RCX+0, DW_OP_constu 0xff, DW_OP_and, DW_OP_convert (0x00000095) "DW_ATE_unsigned_1", DW_OP_convert (0x0000009c) "DW_ATE_unsigned_8", DW_OP_stack_value)
> +DSYM:                   DW_AT_name    ("b")
> +DSYM:                   DW_AT_type    (0x000000da "_Bool")
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits