Page MenuHomePhabricator

[DebugInfo][1/N] Instruction referencing test changes: DBG_VALUE to DBG_INSTR_REF

Authored by jmorse on Nov 19 2021, 9:48 AM.



This is the first patch in a series of test updates, to change X86 tests to look for instruction referencing variable locations. This patch contains a bunch of replacements of:

DBG_VALUE $somereg


SOMEINST debug-instr-number1
DBG_INSTR_REF 1, 0, ...

It's mostly SelectionDAG tests that are making sure that the variable location assignment is placed in the correct position in the instructions.

Diff Detail

Unit TestsFailed

5,440 msx64 debian > libFuzzer.libFuzzer::fork-ubsan.test
Script: -- : 'RUN: at line 3'; /var/lib/buildkite-agent/builds/llvm-project/build/./bin/clang --driver-mode=g++ -O2 -gline-tables-only -fsanitize=address,fuzzer -I/var/lib/buildkite-agent/builds/llvm-project/compiler-rt/lib/fuzzer -m64 /var/lib/buildkite-agent/builds/llvm-project/compiler-rt/test/fuzzer/IntegerOverflowTest.cpp -o /var/lib/buildkite-agent/builds/llvm-project/build/projects/compiler-rt/test/fuzzer/X86_64DefaultLinuxConfig/Output/fork-ubsan.test.tmp-IntegerOverflowTest -fsanitize=signed-integer-overflow -fno-sanitize-recover=signed-integer-overflow

Event Timeline

jmorse created this revision.Nov 19 2021, 9:48 AM
jmorse requested review of this revision.Nov 19 2021, 9:48 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 19 2021, 9:48 AM
jmorse added inline comments.Nov 19 2021, 9:59 AM

Added this because various parts of instruction referencing try to skip doing any work if there are no instructions in scope -- and no "real" instructions in this function are in scope.

djtodoro added inline comments.Nov 22 2021, 3:01 AM

Are we losing the test coverage for the DBG_VALUEs by doing this? I guess it is OK for the targets that will use this feature ON by default, right?

jmorse added inline comments.Nov 22 2021, 3:40 AM

Yes and no -- the debug instruction placement code is shared between instruction referencing and DBG_VALUEs, so it's sort-of being covered. On the other hand, that might change, and I don't think any other architectures cover these parts of SelectionDAG.

I suppose I can leave the CHECK lines in from DBG_VALUE mode, and it's not going to harm anything / over-test, and that'll ensure that SelectionDAG is doing the right thing in DBG_VALUE mode, does that sound alright? (I'd prefer to not do that for the tests in the other patches)

jmorse added inline comments.Nov 22 2021, 3:45 AM

(... and to be clear, that preference is because as far as I'm aware, SelectionDAG is the only thing really shared between DBG_VALUE and instr-ref)

djtodoro added inline comments.Nov 22 2021, 4:23 AM

OK, sounds good to me. Let's add two checks for both modes in SelectionDAG.

jmorse updated this revision to Diff 389495.Nov 24 2021, 7:37 AM

Updated these tests to keep the DBG_VALUE outputs for SelectionDAG outputs.

This revision was not accepted when it landed; it landed in state Needs Review.Nov 29 2021, 1:13 PM
This revision was automatically updated to reflect the committed changes.