[MC] Pass the section to onSymbolStart() for more context

[rochauha]

Summary

This allows the disassembler to have more context regarding how to treat a particular symbol during disassembly.

[MaskRay]

Can you demonstrate how you are going to use the new interface?

llvm/include/llvm/MC/MCDisassembler/MCDisassembler.h

156

Pass object::SectionRef by value. It has 2 words, which is cheaper to copy.

[rochauha]

In D83801#2152421, @MaskRay wrote:

Can you demonstrate how you are going to use the new interface?

The note section in AMDGPU ELF objects contains metadata. But we need to know whether the section is note section to deal with it.

[jhenderson]

In D83801#2152452, @rochauha wrote:

In D83801#2152421, @MaskRay wrote:

Can you demonstrate how you are going to use the new interface?

The note section in AMDGPU ELF objects contains metadata. But we need to know whether the section is note section to deal with it.

I'm not sure I follow. Are you saying you need information from the note section to disassemble other sections? Or are you saying you need it to disassemble the note section? If the former, there's already code in the ELFDumper of llvm-readobj for parsing AMDGPU note sections, which you could reuse (it would need moving somewhere outside the llvm-readobj tool into one of the libraries, maybe in the ELFObjectFile stuff, I'm not sure). If the latter, there's still opposition to adding the functionality to the disassembler (the previous concerns haven't been addressed), given that llvm-readobj already provides this functionality.

[rochauha]

In D83801#2152551, @jhenderson wrote:

In D83801#2152452, @rochauha wrote:

In D83801#2152421, @MaskRay wrote:

Can you demonstrate how you are going to use the new interface?

The note section in AMDGPU ELF objects contains metadata. But we need to know whether the section is note section to deal with it.

I'm not sure I follow. Are you saying you need information from the note section to disassemble other sections? Or are you saying you need it to disassemble the note section? If the former, there's already code in the ELFDumper of llvm-readobj for parsing AMDGPU note sections, which you could reuse (it would need moving somewhere outside the llvm-readobj tool into one of the libraries, maybe in the ELFObjectFile stuff, I'm not sure). If the latter, there's still opposition to adding the functionality to the disassembler (the previous concerns haven't been addressed), given that llvm-readobj already provides this functionality.

There is AMDGPU metadata stored in the note section, which needs to be disassembled. The SectionRef passed here is just to check whether the section is a note section. There is no iterating over the note section.

This work doesn't fundamentally alter the the infrastructure. It just provides additional context to onSymbolStart (which some other target might probably find useful in the future too?).

[jhenderson]

Like @MaskRay, I'm going to see more usage of this to be able to properly comment on this change itself. However, one major point that needs highlighting is that this introduces a dependency on the Object library from the MC Disassembler, which doesn't currently exist. As far as I can tell, there are no references from MC or MCDisassembler to Object, but there are the other way around. That probably means this approach is a no-go. On the other hand, there is a SectionKind.h header file in MC, which might be an appropriate thing to use/extend if you need to identify the type of a section.

llvm/include/llvm/MC/MCDisassembler/MCDisassembler.h

 //===- llvm/MC/MCDisassembler.h - Disassembler interface --------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_MC_MCDISASSEMBLER_MCDISASSEMBLER_H
 #define LLVM_MC_MCDISASSEMBLER_MCDISASSEMBLER_H
 
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/BinaryFormat/XCOFF.h"
 #include "llvm/MC/MCDisassembler/MCSymbolizer.h"
+#include "llvm/Object/ObjectFile.h"
 #include <cstdint>
 #include <memory>
 #include <vector>
 
 namespace llvm {
 
 struct XCOFFSymbolInfo {
   Optional<XCOFF::StorageMappingClass> StorageMappingClass;
public:
   /// symbol. May parse any prelude that precedes instructions after the
   /// start of a symbol, or the entire symbol.
   /// This is used for example by WebAssembly to decode preludes.
   ///
   /// Base implementation returns None. So all targets by default ignore to
   /// treat symbols separately.
   ///
   /// \param Symbol   - The symbol.
+  /// \param Section  - Section containing the symbol.
   /// \param Size     - The number of bytes consumed.
   /// \param Address  - The address, in the memory space of region, of the first
   ///                   byte of the symbol.
   /// \param Bytes    - A reference to the actual bytes at the symbol location.
   /// \param CStream  - The stream to print comments and annotations on.
   /// \return         - MCDisassembler::Success if bytes are decoded
   ///                   successfully. Size must hold the number of bytes that
   ///                   were decoded.
   ///                 - MCDisassembler::Fail if the bytes are invalid. Size
   ///                   must hold the number of bytes that were decoded before
   ///                   failing. The target must print nothing. This can be
   ///                   done by buffering the output if needed.
   ///                 - None if the target doesn't want to handle the symbol
   ///                   separately. Value of Size is ignored in this case.
   virtual Optional<DecodeStatus>
-  onSymbolStart(SymbolInfoTy &Symbol, uint64_t &Size, ArrayRef<uint8_t> Bytes,
-                uint64_t Address, raw_ostream &CStream) const;
+  onSymbolStart(SymbolInfoTy &Symbol, const object::SectionRef &Section,
+                uint64_t &Size, ArrayRef<uint8_t> Bytes, uint64_t Address,
+                raw_ostream &CStream) const;
   // TODO:
   // Implement similar hooks that can be used at other points during
   // disassembly. Something along the following lines:
   // - onBeforeInstructionDecode()
   // - onAfterInstructionDecode()
   // - onSymbolEnd()
   // It should help move much of the target specific code from llvm-objdump to
   // respective target disassemblers.

llvm/lib/MC/MCDisassembler/MCDisassembler.cpp

 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/raw_ostream.h"
 #include <algorithm>
 
 using namespace llvm;
 
 MCDisassembler::~MCDisassembler() = default;
 
-Optional<MCDisassembler::DecodeStatus>
-MCDisassembler::onSymbolStart(SymbolInfoTy &Symbol, uint64_t &Size,
-                              ArrayRef<uint8_t> Bytes, uint64_t Address,
-                              raw_ostream &CStream) const {
+Optional<MCDisassembler::DecodeStatus> MCDisassembler::onSymbolStart(
+    SymbolInfoTy &Symbol, const object::SectionRef &Section, uint64_t &Size,
+    ArrayRef<uint8_t> Bytes, uint64_t Address, raw_ostream &CStream) const {
   return None;
 }
 
 bool MCDisassembler::tryAddingSymbolicOperand(MCInst &Inst, int64_t Value,
                                               uint64_t Address, bool IsBranch,
                                               uint64_t Offset,
                                               uint64_t InstSize) const {
   if (Symbolizer)

llvm/lib/Target/WebAssembly/Disassembler/WebAssemblyDisassembler.cpp

 static constexpr int WebAssemblyInstructionTableSize = 256;
 
 class WebAssemblyDisassembler final : public MCDisassembler {
   std::unique_ptr<const MCInstrInfo> MCII;
 
   DecodeStatus getInstruction(MCInst &Instr, uint64_t &Size,
                               ArrayRef<uint8_t> Bytes, uint64_t Address,
                               raw_ostream &CStream) const override;
-  Optional<DecodeStatus> onSymbolStart(SymbolInfoTy &Symbol, uint64_t &Size,
-                                       ArrayRef<uint8_t> Bytes,
+  Optional<DecodeStatus> onSymbolStart(SymbolInfoTy &Symbol,
+                                       const object::SectionRef &Section,
+                                       uint64_t &Size, ArrayRef<uint8_t> Bytes,
                                        uint64_t Address,
                                        raw_ostream &CStream) const override;
 
 public:
   WebAssemblyDisassembler(const MCSubtargetInfo &STI, MCContext &Ctx,
                           std::unique_ptr<const MCInstrInfo> MCII)
       : MCDisassembler(STI, Ctx), MCII(std::move(MCII)) {}
 };
if (std::is_floating_point<T>::value) {
     MI.addOperand(MCOperand::createFPImm(static_cast<double>(Val)));
   } else {
     MI.addOperand(MCOperand::createImm(static_cast<int64_t>(Val)));
   }
   return true;
 }
 
 Optional<MCDisassembler::DecodeStatus> WebAssemblyDisassembler::onSymbolStart(
-    SymbolInfoTy &Symbol, uint64_t &Size, ArrayRef<uint8_t> Bytes,
-    uint64_t Address, raw_ostream &CStream) const {
+    SymbolInfoTy &Symbol, const object::SectionRef &Section, uint64_t &Size,
+    ArrayRef<uint8_t> Bytes, uint64_t Address, raw_ostream &CStream) const {
   Size = 0;
   if (Address == 0) {
     // Start of a code section: we're parsing only the function count.
     int64_t FunctionCount;
     if (!nextLEB(FunctionCount, Bytes, Size, false))
       return None;
     outs() << "        # " << FunctionCount << " functions in section.";
   } else {

llvm/tools/llvm-objdump/llvm-objdump.cpp

for (unsigned SI = 0, SE = Symbols.size(); SI != SE; ++SI) {
 
       // Don't print raw contents of a virtual section. A virtual section
       // doesn't have any contents in the file.
       if (Section.isVirtual()) {
         outs() << "...\n";
         continue;
       }
 
-      auto Status = DisAsm->onSymbolStart(Symbols[SI], Size,
+      auto Status = DisAsm->onSymbolStart(Symbols[SI], Section, Size,
                                           Bytes.slice(Start, End - Start),
                                           SectionAddr + Start, CommentStream);
       // To have round trippable disassembly, we fall back to decoding the
       // remaining bytes as instructions.
       //
       // If there is a failure, we disassemble the failed region as bytes before
       // falling back. The target is expected to print nothing in this case.
       //