[SeaBIOS] [PATCH v3] Replace $(OBJDUMP) -thr with $(READELF) -WSrs

Fangrui Song via SeaBIOS posted 1 patch 2 months, 2 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/seabios tags/patchew/20220713023744.3451795-1-maskray@google.com
Makefile             |  18 +++---
scripts/checkrom.py  |   2 +-
scripts/layoutrom.py | 145 ++++++++++++++++++++-----------------------
3 files changed, 79 insertions(+), 86 deletions(-)
[SeaBIOS] [PATCH v3] Replace $(OBJDUMP) -thr with $(READELF) -WSrs
Posted by Fangrui Song via SeaBIOS 2 months, 2 weeks ago
objdump -h relies heavily on BFD internals and the BFD flags. The output
is difficult to emulate in llvm-objdump. (llvm-objdump -h has a
different output https://reviews.llvm.org/D57146). Switch to READELF, so
that llvm-readelf can be used as an alternative.

readelf is generally better than objdump when dumping ELF section/symbol
information.

Signed-off-by: Fangrui Song <maskray@google.com>
---
Changes from v2 (https://mail.coreboot.org/hyperkitty/list/seabios@seabios.org/message/4AZCP2KR63YAURID5FFXOWMPIOB4BNRJ/)
* Change an objdump comment to mention readelf
* For your convenience, this patch is available on https://github.com/MaskRay/seabios/tree/readelf
---
 Makefile             |  18 +++---
 scripts/checkrom.py  |   2 +-
 scripts/layoutrom.py | 145 ++++++++++++++++++++-----------------------
 3 files changed, 79 insertions(+), 86 deletions(-)

diff --git a/Makefile b/Makefile
index c108f87..5c6a838 100644
--- a/Makefile
+++ b/Makefile
@@ -20,7 +20,7 @@ endif
 AS=$(CROSS_PREFIX)as
 LD=$(CROSS_PREFIX)ld
 OBJCOPY=$(CROSS_PREFIX)objcopy
-OBJDUMP=$(CROSS_PREFIX)objdump
+READELF=$(CROSS_PREFIX)readelf
 STRIP=$(CROSS_PREFIX)strip
 PYTHON=python
 CPP=cpp
@@ -165,14 +165,14 @@ $(OUT)romlayout.o: src/romlayout.S $(OUT)autoconf.h $(OUT)asm-offsets.h
 
 $(OUT)romlayout16.lds: $(OUT)ccode32flat.o $(OUT)code32seg.o $(OUT)ccode16.o $(OUT)romlayout.o src/version.c scripts/layoutrom.py scripts/buildversion.py
 	@echo "  Building ld scripts"
-	$(Q)$(PYTHON) ./scripts/buildversion.py -e "$(EXTRAVERSION)" -t "$(CC);$(AS);$(LD);$(OBJCOPY);$(OBJDUMP);$(STRIP)" $(OUT)autoversion.h
+	$(Q)$(PYTHON) ./scripts/buildversion.py -e "$(EXTRAVERSION)" -t "$(CC);$(AS);$(LD);$(OBJCOPY);$(READELF);$(STRIP)" $(OUT)autoversion.h
 	$(Q)$(CC) $(CFLAGS32FLAT) -c src/version.c -o $(OUT)version.o
 	$(Q)$(LD) $(LD32BIT_FLAG) -r $(OUT)ccode32flat.o $(OUT)version.o -o $(OUT)code32flat.o
 	$(Q)$(LD) $(LD32BIT_FLAG) -r $(OUT)ccode16.o $(OUT)romlayout.o -o $(OUT)code16.o
-	$(Q)$(OBJDUMP) -thr $(OUT)code32flat.o > $(OUT)code32flat.o.objdump
-	$(Q)$(OBJDUMP) -thr $(OUT)code32seg.o > $(OUT)code32seg.o.objdump
-	$(Q)$(OBJDUMP) -thr $(OUT)code16.o > $(OUT)code16.o.objdump
-	$(Q)$(PYTHON) ./scripts/layoutrom.py $(OUT)code16.o.objdump $(OUT)code32seg.o.objdump $(OUT)code32flat.o.objdump $(OUT)$(KCONFIG_AUTOHEADER) $(OUT)romlayout16.lds $(OUT)romlayout32seg.lds $(OUT)romlayout32flat.lds
+	$(Q)$(READELF) -WSrs $(OUT)code32flat.o > $(OUT)code32flat.o.readelf
+	$(Q)$(READELF) -WSrs $(OUT)code32seg.o > $(OUT)code32seg.o.readelf
+	$(Q)$(READELF) -WSrs $(OUT)code16.o > $(OUT)code16.o.readelf
+	$(Q)$(PYTHON) ./scripts/layoutrom.py $(OUT)code16.o.readelf $(OUT)code32seg.o.readelf $(OUT)code32flat.o.readelf $(OUT)$(KCONFIG_AUTOHEADER) $(OUT)romlayout16.lds $(OUT)romlayout32seg.lds $(OUT)romlayout32flat.lds
 
 # These are actually built by scripts/layoutrom.py above, but by pulling them
 # into an extra rule we prevent make -j from spawning layoutrom.py 4 times.
@@ -193,9 +193,9 @@ $(OUT)rom.o: $(OUT)rom16.noexec.o $(OUT)rom32seg.noexec.o $(OUT)code32flat.o $(O
 $(OUT)bios.bin.prep: $(OUT)rom.o scripts/checkrom.py
 	@echo "  Prepping $@"
 	$(Q)rm -f $(OUT)bios.bin $(OUT)Csm16.bin $(OUT)bios.bin.elf
-	$(Q)$(OBJDUMP) -thr $< > $<.objdump
+	$(Q)$(READELF) -WSrs $< > $<.readelf
 	$(Q)$(OBJCOPY) -O binary $< $(OUT)bios.bin.raw
-	$(Q)$(PYTHON) ./scripts/checkrom.py $<.objdump $(CONFIG_ROM_SIZE) $(OUT)bios.bin.raw $(OUT)bios.bin.prep
+	$(Q)$(PYTHON) ./scripts/checkrom.py $<.readelf $(CONFIG_ROM_SIZE) $(OUT)bios.bin.raw $(OUT)bios.bin.prep
 
 $(OUT)bios.bin: $(OUT)bios.bin.prep
 	@echo "  Creating $@"
@@ -237,7 +237,7 @@ $(OUT)vgaentry.o: vgasrc/vgaentry.S $(OUT)autoconf.h $(OUT)asm-offsets.h
 
 $(OUT)vgarom.o: $(OUT)vgaccode16.o $(OUT)vgaentry.o $(OUT)vgasrc/vgalayout.lds vgasrc/vgaversion.c scripts/buildversion.py
 	@echo "  Linking $@"
-	$(Q)$(PYTHON) ./scripts/buildversion.py -e "$(EXTRAVERSION)" -t "$(CC);$(AS);$(LD);$(OBJCOPY);$(OBJDUMP);$(STRIP)" $(OUT)autovgaversion.h
+	$(Q)$(PYTHON) ./scripts/buildversion.py -e "$(EXTRAVERSION)" -t "$(CC);$(AS);$(LD);$(OBJCOPY);$(READELF);$(STRIP)" $(OUT)autovgaversion.h
 	$(Q)$(CC) $(CFLAGS16) -c vgasrc/vgaversion.c -o $(OUT)vgaversion.o
 	$(Q)$(LD) --gc-sections -T $(OUT)vgasrc/vgalayout.lds $(OUT)vgaccode16.o $(OUT)vgaentry.o $(OUT)vgaversion.o -o $@
 
diff --git a/scripts/checkrom.py b/scripts/checkrom.py
index a5b15a4..4a23f0a 100755
--- a/scripts/checkrom.py
+++ b/scripts/checkrom.py
@@ -21,7 +21,7 @@ def main():
 
     # Read in symbols
     objinfofile = open(objinfo, 'r')
-    symbols = layoutrom.parseObjDump(objinfofile, 'in')[1]
+    symbols = layoutrom.parseReadElf(objinfofile, 'in')[1]
 
     # Read in raw file
     f = open(rawfile, 'rb')
diff --git a/scripts/layoutrom.py b/scripts/layoutrom.py
index 7bc4a57..f4b94ea 100755
--- a/scripts/layoutrom.py
+++ b/scripts/layoutrom.py
@@ -541,87 +541,80 @@ class Reloc:
 class Symbol:
     name = offset = section = None
 
-# Read in output from objdump
-def parseObjDump(file, fileid):
+def parseReadElf(file, fileid):
     # sections = [section, ...]
     sections = []
     sectionmap = {}
     # symbols[symbolname] = symbol
     symbols = {}
+    lines = file.readlines()
 
-    state = None
-    for line in file.readlines():
-        line = line.rstrip()
-        if line == 'Sections:':
-            state = 'section'
-            continue
-        if line == 'SYMBOL TABLE:':
-            state = 'symbol'
-            continue
-        if line.startswith('RELOCATION RECORDS FOR ['):
-            sectionname = line[24:-2]
-            if sectionname.startswith('.debug_'):
-                # Skip debugging sections (to reduce parsing time)
-                state = None
-                continue
-            state = 'reloc'
-            relocsection = sectionmap[sectionname]
-            continue
+    i = 0
+    while not lines[i].startswith('Section Headers'):
+        i += 1
+    i += 3  # Skip [Nr] and [ 0]
+    while ']' in lines[i]:
+        parts = lines[i][lines[i].find(']')+1:].split()
+        if len(parts) == 10:
+            name, _, _, _, size, _, _, _, _, align = parts
+        else:  # not SHF_ALLOC
+            name, _, _, _, size, _, _, _, align = parts
+        section = Section()
+        section.name = name
+        section.size = int(size, 16)
+        section.align = int(align)
+        section.fileid = fileid
+        section.relocs = []
+        sections.append(section)
+        sectionmap[name] = section
+        i += 1
 
-        if state == 'section':
-            try:
-                idx, name, size, vma, lma, fileoff, align = line.split()
-                if align[:3] != '2**':
-                    continue
-                section = Section()
-                section.name = name
-                section.size = int(size, 16)
-                section.align = 2**int(align[3:])
-                section.fileid = fileid
-                section.relocs = []
-                sections.append(section)
-                sectionmap[name] = section
-            except ValueError:
-                pass
+    while not lines[i].startswith('Symbol table '):
+        i += 1
+    i += 3  # Skip Num: and 0:
+    while i < len(lines) and ':' in lines[i]:
+        colon = lines[i].find(':')
+        parts = lines[i].split()
+        try:
+            ndx = int(parts[6])
+            name = sections[ndx-1].name
+        except ValueError:  # ABS or UND
+            ndx = 0
+            name = ''
+        if parts[3] != 'SECTION':
+            name = parts[7]
+        symbol = Symbol()
+        symbol.size = int(parts[2])
+        symbol.offset = int(parts[1], 16)
+        symbol.name = name
+        symbol.section = sections[ndx-1] if ndx > 0 else None
+        symbols[name] = symbol
+        i += 1
+
+    i = 0
+    while i < len(lines):
+        if not lines[i].startswith('Relocation section'):
+            i += 1
             continue
-        if state == 'symbol':
-            try:
-                parts = line[17:].split()
-                if len(parts) == 3:
-                    sectionname, size, name = parts
-                elif len(parts) == 4 and parts[2] == '.hidden':
-                    sectionname, size, hidden, name = parts
-                else:
-                    continue
-                symbol = Symbol()
-                symbol.size = int(size, 16)
-                symbol.offset = int(line[:8], 16)
-                symbol.name = name
-                symbol.section = sectionmap.get(sectionname)
-                symbols[name] = symbol
-            except ValueError:
-                pass
+        rel_name = lines[i][20:lines[i].index("'", 20)]
+        i += 2
+        if rel_name.startswith('.rel.debug_'):
+            # Skip debugging sections (to reduce parsing time)
             continue
-        if state == 'reloc':
-            try:
-                off, type, symbolname = line.split()
-                reloc = Reloc()
-                reloc.offset = int(off, 16)
-                reloc.type = type
-                reloc.symbolname = symbolname
-                reloc.symbol = symbols.get(symbolname)
-                if reloc.symbol is None:
-                    # Some binutils (2.20.1) give section name instead
-                    # of a symbol - create a dummy symbol.
-                    reloc.symbol = symbol = Symbol()
-                    symbol.size = 0
-                    symbol.offset = 0
-                    symbol.name = symbolname
-                    symbol.section = sectionmap.get(symbolname)
-                    symbols[symbolname] = symbol
-                relocsection.relocs.append(reloc)
-            except ValueError:
-                pass
+        assert rel_name.startswith('.rel')
+        relocated = sectionmap[rel_name[4:]]
+        while lines[i][0].isalnum():
+            line = lines[i]
+            off, _, typ, _, symbolname = line.split()
+            reloc = Reloc()
+            reloc.offset = int(off, 16)
+            reloc.type = typ
+            reloc.symbolname = symbolname
+            reloc.symbol = symbols.get(symbolname)
+            assert reloc.symbol is not None
+            relocated.relocs.append(reloc)
+            i += 1
+
     return sections, symbols
 
 # Parser for constants in simple C header files.
@@ -644,15 +637,15 @@ def main():
     # Get output name
     in16, in32seg, in32flat, cfgfile, out16, out32seg, out32flat = sys.argv[1:]
 
-    # Read in the objdump information
+    # Read in the readelf information
     infile16 = open(in16, 'r')
     infile32seg = open(in32seg, 'r')
     infile32flat = open(in32flat, 'r')
 
     # infoX = (sections, symbols)
-    info16 = parseObjDump(infile16, '16')
-    info32seg = parseObjDump(infile32seg, '32seg')
-    info32flat = parseObjDump(infile32flat, '32flat')
+    info16 = parseReadElf(infile16, '16')
+    info32seg = parseReadElf(infile32seg, '32seg')
+    info32flat = parseReadElf(infile32flat, '32flat')
 
     # Read kconfig config file
     config = scanconfig(cfgfile)
-- 
2.37.0.144.g8ac04bfd2-goog

_______________________________________________
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-leave@seabios.org
[SeaBIOS] Re: [PATCH v3] Replace $(OBJDUMP) -thr with $(READELF) -WSrs
Posted by Paul Menzel 2 months, 2 weeks ago
Dear Fangrui,


Thank you for the patch.

Am 13.07.22 um 04:37 schrieb Fangrui Song via SeaBIOS:
> objdump -h relies heavily on BFD internals and the BFD flags. The output
> is difficult to emulate in llvm-objdump. (llvm-objdump -h has a
> different output https://reviews.llvm.org/D57146). Switch to READELF, so

A small nit: I’d put the dot inside the brackets, as the sentence is in 
the brackets.

> that llvm-readelf can be used as an alternative.

[…]


Kind regards,

Paul
_______________________________________________
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-leave@seabios.org
[SeaBIOS] Re: [PATCH v3] Replace $(OBJDUMP) -thr with $(READELF) -WSrs
Posted by Fangrui Song via SeaBIOS 2 months, 2 weeks ago
On Wed, Jul 13, 2022 at 10:01 PM Paul Menzel <pmenzel@molgen.mpg.de> wrote:
>
> Dear Fangrui,
>
>
> Thank you for the patch.
>
> Am 13.07.22 um 04:37 schrieb Fangrui Song via SeaBIOS:
> > objdump -h relies heavily on BFD internals and the BFD flags. The output
> > is difficult to emulate in llvm-objdump. (llvm-objdump -h has a
> > different output https://reviews.llvm.org/D57146). Switch to READELF, so
>
> A small nit: I’d put the dot inside the brackets, as the sentence is in
> the brackets.

Thanks. I amended the commit message on
https://github.com/MaskRay/seabios/tree/readelf
I keep a space after the URI, otherwise some terminals may treat the
`.` as part of the URI.

> > that llvm-readelf can be used as an alternative.
>
> […]
>
>
> Kind regards,
>
> Paul
_______________________________________________
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-leave@seabios.org