Skip to content

Conversation

quic-areg
Copy link
Contributor

Hexagon packets can visually straddle labels when data (e.g. jump tables) in the text section does not carry end-of-packet bits. In such cases the next instruction, even at a new symbol, appears to continue the previous packet.

This patch resets packet state when encountering a new symbol so that packets at symbol starts are guaranteed to start in their own packet.

@llvmbot
Copy link
Member

llvmbot commented Oct 14, 2025

@llvm/pr-subscribers-llvm-binary-utilities

@llvm/pr-subscribers-backend-hexagon

Author: None (quic-areg)

Changes

Hexagon packets can visually straddle labels when data (e.g. jump tables) in the text section does not carry end-of-packet bits. In such cases the next instruction, even at a new symbol, appears to continue the previous packet.

This patch resets packet state when encountering a new symbol so that packets at symbol starts are guaranteed to start in their own packet.


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

3 Files Affected:

  • (modified) llvm/lib/Target/Hexagon/Disassembler/HexagonDisassembler.cpp (+16)
  • (added) llvm/test/tools/llvm-objdump/ELF/Hexagon/packet-reset-on-label.s (+23)
  • (modified) llvm/tools/llvm-objdump/llvm-objdump.cpp (+8)
diff --git a/llvm/lib/Target/Hexagon/Disassembler/HexagonDisassembler.cpp b/llvm/lib/Target/Hexagon/Disassembler/HexagonDisassembler.cpp
index 5f180d6448d06..3bd6ed4cbae86 100644
--- a/llvm/lib/Target/Hexagon/Disassembler/HexagonDisassembler.cpp
+++ b/llvm/lib/Target/Hexagon/Disassembler/HexagonDisassembler.cpp
@@ -66,6 +66,10 @@ class HexagonDisassembler : public MCDisassembler {
 
   void remapInstruction(MCInst &Instr) const;
 
+  Expected<bool> onSymbolStart(SymbolInfoTy &Symbol, uint64_t &Size,
+                               ArrayRef<uint8_t> Bytes,
+                               uint64_t Address) const override;
+
 private:
   bool makeBundle(ArrayRef<uint8_t> Bytes, uint64_t Address,
                   uint64_t &BytesToSkip, raw_ostream &CS) const;
@@ -567,6 +571,18 @@ DecodeStatus HexagonDisassembler::getSingleInstruction(MCInst &MI, MCInst &MCB,
   return Result;
 }
 
+Expected<bool> HexagonDisassembler::onSymbolStart(SymbolInfoTy &Symbol,
+                                                  uint64_t &Size,
+                                                  ArrayRef<uint8_t> Bytes,
+                                                  uint64_t Address) const {
+  // At the start of a symbol, force a fresh packet by resetting any
+  // in-progress bundle state. This prevents packets from straddling label
+  // boundaries when data (e.g. jump tables) appears in between.
+  Size = 0;
+  resetBundle();
+  return true;
+}
+
 static DecodeStatus DecodeRegisterClass(MCInst &Inst, unsigned RegNo,
                                         ArrayRef<MCPhysReg> Table) {
   if (RegNo < Table.size()) {
diff --git a/llvm/test/tools/llvm-objdump/ELF/Hexagon/packet-reset-on-label.s b/llvm/test/tools/llvm-objdump/ELF/Hexagon/packet-reset-on-label.s
new file mode 100644
index 0000000000000..02a52bbb3fbd8
--- /dev/null
+++ b/llvm/test/tools/llvm-objdump/ELF/Hexagon/packet-reset-on-label.s
@@ -0,0 +1,23 @@
+// RUN: llvm-mc -triple=hexagon -mcpu=hexagonv75 -filetype=obj %s \
+// RUN:   | llvm-objdump -d - \
+// RUN:   | FileCheck %s
+
+foo:
+  { nop }
+  /// a nop without end-of-packet bits set to simulate data that is
+  /// not a proper packet end.
+  .long 0x7f004000
+bar:
+  { nop
+    nop
+  }
+
+// CHECK-LABEL: <foo>:
+// CHECK: { nop }
+// CHECK-NEXT: { nop
+
+/// The instruction starting after <bar> should start in a new packet.
+// CHECK-LABEL: <bar>:
+// CHECK: { nop
+// CHECK-NEXT: nop }
+
diff --git a/llvm/tools/llvm-objdump/llvm-objdump.cpp b/llvm/tools/llvm-objdump/llvm-objdump.cpp
index 46be539d6f9e6..3ec644a472bfc 100644
--- a/llvm/tools/llvm-objdump/llvm-objdump.cpp
+++ b/llvm/tools/llvm-objdump/llvm-objdump.cpp
@@ -728,11 +728,17 @@ class PrettyPrinter {
     } while (!Comments.empty());
     FOS.flush();
   }
+
+  // Hook invoked when starting to disassemble a symbol at the current position.
+  // Default is no-op.
+  virtual void onSymbolStart() {}
 };
 PrettyPrinter PrettyPrinterInst;
 
 class HexagonPrettyPrinter : public PrettyPrinter {
 public:
+  void onSymbolStart() override { reset(); }
+
   void printLead(ArrayRef<uint8_t> Bytes, uint64_t Address,
                  formatted_raw_ostream &OS) {
     if (LeadingAddr)
@@ -2228,6 +2234,8 @@ disassembleObject(ObjectFile &Obj, const ObjectFile &DbgObj,
         Start += Size;
         break;
       }
+      // Allow targets to reset any per-symbol state.
+      DT->Printer->onSymbolStart();
       formatted_raw_ostream FOS(OS);
       Index = Start;
       if (SectionAddr < StartAddress)

Hexagon packets can visually straddle labels when data (e.g. jump tables)
in the text section does not carry end-of-packet bits. In such cases
the next  instruction, even at a new symbol, appears to continue the
previous packet.

This patch resets packet state when encoutering a new symbol so that
packets at symbol starts are guaranteed to start in their own packet.
@quic-areg quic-areg merged commit 0cdebda into llvm:main Oct 16, 2025
10 checks passed
@quic-areg quic-areg deleted the reset-on-label branch October 16, 2025 00:11
@quic-areg quic-areg added this to the LLVM 21.x Release milestone Oct 16, 2025
@github-project-automation github-project-automation bot moved this to Needs Triage in LLVM Release Status Oct 16, 2025
@quic-areg
Copy link
Contributor Author

/cherry-pick 0cdebda

@llvmbot
Copy link
Member

llvmbot commented Oct 16, 2025

/pull-request #163662

@llvmbot llvmbot moved this from Needs Triage to Done in LLVM Release Status Oct 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Development

Successfully merging this pull request may close these issues.

3 participants