[llvm-profgen] Fix an out-of-range error during unwinding

[wlei]

Summary

It happened that the LBR entry target can be the first address of text section which causes an out-of-range crash. So here add a boundary check.

[wlei]

fix typo

[wenlei]

llvm/tools/llvm-profgen/PerfReader.cpp

63–75

Why do we run into such cases? Is the profile corrupted? If that's the case, it may make sense to emit a warning about corrupted profile.

As for the check itself, how about fold it into advance/backward, and let them return a boolean for caller to check? Index is implementation details of InstructionPoint and would be good to avoid using it from outside.

[wenlei]

llvm/tools/llvm-profgen/PerfReader.cpp

63–75

Actually Index==0 is still valid, the problem is we cannot "backward" from index 0. However we still want to "recordRangeCount" if this index 0 is the end of the linear range. But the fix would skip "recordRangeCount" in end of range has index 0, which looks incorrect. Or am I missing something?

[wlei]

Updating D110271: [llvm-profgen] Fix an out-of-range error during unwinding

[wlei]

llvm/tools/llvm-profgen/PerfReader.cpp

63–75

Good point, Index 0 is missed then. The while loop here did compare the IP and next IP to see if they're in the same inline context or if we reach the end(target). I think the mixed logic is a little confusing. so I separate the logic letting the while loop only for inline check and after the loop we know it's the end and record the remaining range.

BTW, some refactors is done for the address being used in switchToFrame, we can use any of the address in the range, so just use the End.

[wenlei]

llvm/tools/llvm-profgen/PerfReader.cpp

81

This is a change beyond refactoring? Before, we use last address of old frame as frame address, but now it's going to be the first address of old frame.

State.switchToFrame(LeafAddr);
->
State.switchToFrame(End);

Or do did you actually meant State.switchToFrame(PrevIP);?

[wlei]

llvm/tools/llvm-profgen/PerfReader.cpp

81

It won't affect the result. I think any address in [PrevIP, End] is good for the leaf frame because we will trim the location info of the last frame. Using End is because End is also used out of the loop(just for readability).

[hoy]

lgtm, thanks for the fix.

[wenlei]

lgtm, thanks.

llvm/tools/llvm-profgen/PerfReader.cpp

81

Ok, so it changes what's used in the frame trie, but as long as we use a consistent address to represent frame, changes there won't affect output, correct?

[wlei]

llvm/tools/llvm-profgen/PerfReader.cpp

81

Sorry, my fault, it should be the last address of the old frame, otherwise if the last address is a call, it will be recorded in the wrong callsite. Thanks for correcting me!

[wlei]

llvm/tools/llvm-profgen/PerfReader.cpp

81

OK, I double checked the call address won't use the address from linear unwinder, it use the LBR address. So changing to End should be fine. Anyway I choose use the last address which align to the program execution logic.

[wlei]

Updating D110271: [llvm-profgen] Fix an out-of-range error during unwinding

llvm/tools/llvm-profgen/PerfReader.cpp

if (Binary->usePseudoProbes()) {
     // from the range.
     // The outcome of the virtual unwinding with pseudo probes is a
     // map from a context key to the address range being unwound.
     // This means basically linear unwinding is not needed for pseudo
     // probes. The range will be simply recorded here and will be
     // converted to a list of pseudo probes to report in ProfileGenerator.
     State.getParentFrame()->recordRangeCount(Target, End, Repeat);
   } else {
-    // Unwind linear execution part
-    uint64_t LeafAddr = State.CurrentLeafFrame->Address;
-    while (IP.Address >= Target) {
+    // Unwind linear execution part.
+    // Split and record the range by different inline context. For example:
+    // [0x01] ... main:1          # Target
+    // [0x02] ... main:2
+    // [0x03] ... main:3 @ foo:1
+    // [0x04] ... main:3 @ foo:2
+    // [0x05] ... main:3 @ foo:3
+    // [0x06] ... main:4
+    // [0x07] ... main:5          # End
+    // It will be recorded:
+    // [main:*]         : [0x06, 0x07], [0x01, 0x02]
+    // [main:3 @ foo:*] : [0x03, 0x05]
+    while (IP.Address > Target) {
       uint64_t PrevIP = IP.Address;
       IP.backward();
       // Break into segments for implicit call/return due to inlining
       bool SameInlinee = Binary->inlineContextEqual(PrevIP, IP.Address);
-      if (!SameInlinee || PrevIP == Target) {
-        State.switchToFrame(LeafAddr);
+      if (!SameInlinee) {
+        State.switchToFrame(PrevIP);
         State.CurrentLeafFrame->recordRangeCount(PrevIP, End, Repeat);
         End = IP.Address;
       }
-      LeafAddr = IP.Address;
     }
+    assert(IP.Address == Target && "The last one must be the target address.");
+    // Record the remaining range, [0x01, 0x02] in the example
+    State.switchToFrame(IP.Address);
+    State.CurrentLeafFrame->recordRangeCount(IP.Address, End, Repeat);
   }
 }
 
 void VirtualUnwinder::unwindReturn(UnwindState &State) {
   // Add extra frame as we unwind through the return
   const LBREntry &LBR = State.getCurrentLBR();
   uint64_t CallAddr = Binary->getCallAddrFromFrameAddr(LBR.Target);
   State.switchToFrame(CallAddr);