fix xrelfo on ARM(32) & cross-compile #8421

eqvinox · 2021-04-07T22:05:38Z

some ARM 32bit ABIs use DT_REL, which the elf_py code in clippy previously just ignored
a bug in the python code used the buildhost's pointer size rather than the target's, breaking crosscompile of 32-bit targets on 64-bit hosts

ARM (32-bit) needs DT_REL... and here I was hoping I could avoid the trouble. Fixes: FRRouting#8355 Signed-off-by: David Lamparter <equinox@diac24.net>

This was mistakenly using the host platform's pointer size rather than the ELF file's. Only noticeable when cross compiling... Signed-off-by: David Lamparter <equinox@diac24.net>

eqvinox · 2021-04-07T22:07:10Z

@lucize can you try this PR?

NetDEF-CI · 2021-04-08T01:17:22Z

Continuous Integration Result: SUCCESSFUL

Congratulations, this patch passed basic tests

Tested-by: NetDEF / OpenSourceRouting.org CI System

CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-18247/

This is a comment from an automated CI system.
For questions and feedback in regards to this CI system, please feel free to email
Martin Winter - mwinter (at) opensourcerouting.org.

LabN-CI · 2021-04-08T04:19:11Z

Outdated results 💚

Basic BGPD CI results: SUCCESS, 0 tests failed

_	_
Result	SUCCESS git merge/8421 `b17f302`
Date	04/07/2021
Start	23:38:07
Finish	00:19:08
Run-Time	41:01
Total	1815
Pass	1815
Fail	0
Valgrind-Errors	0
Valgrind-Loss	0
Details	vncregress-2021-04-07-23:38:07.txt
Log	autoscript-2021-04-07-23:39:12.log.bz2
Memory	498 508 427

For details, please contact louberger

lucize · 2021-04-08T08:26:02Z

@eqvinox
Yes, now it's working, also run tested on mips

eqvinox · 2021-04-08T10:19:15Z

@lucize awesome, Thanks for testing!

lucize · 2021-04-08T15:34:32Z

@eqvinox
I tried a clean build and it seems that if I try to reduce size using lto I get this (on arm also)

/home/build/erx/staging_dir/hostpkg/bin/clippy ./python/xrelfo.py  -o watchfrr/watchfrr.xref watchfrr/watchfrr
{standard input}: Assembler messages:
{standard input}:30: Error: symbol `_frr_xref_note' is already defined
lto-wrapper: fatal error: mipsel-openwrt-linux-musl-gcc returned 1 exit status
compilation terminated.

eqvinox · 2021-04-08T16:26:07Z

@lucize those messages aren't from clippy - are you using parallel build & the output is from an earlier command?

Also I think the problem is libfrr being linked statically, are you trying to LTO daemons+libfrr in one piece?

lucize · 2021-04-08T16:39:06Z

So if I use -j1 it seems that it's working

LE:

seems that is related to pathd daemon, if it's disabled lto works

eqvinox · 2021-04-08T18:36:04Z

@lucize can you upload the entire (failing) build log somewhere, with make V=1?

eqvinox · 2021-04-08T18:36:25Z

(either way this seems unrelated to this PR, we should merge this and continue on a new issue)

Can't have things duplicate in libpath.a and pathd directly, they'll crash into eath other on linking. No idea why this doesn't error out in our CI builds, but it definitely breaks LTO builds. Signed-off-by: David Lamparter <equinox@diac24.net>

polychaeta

Thanks for your contribution to FRR!

Pylint found errors in source files changed by this PR:

Pylint report for my_frr/python/clippy/elf.py:
************* Module clippy.elf
my_frr/python/clippy/elf.py:146:31: E1101: Instance of 'ELFDissectData' has no '_data' member (no-member)
my_frr/python/clippy/elf.py:146:66: E1101: Instance of 'ELFDissectData' has no '_data' member (no-member)
my_frr/python/clippy/elf.py:162:37: E1101: Instance of 'ELFDissectData' has no 'elfclass' member (no-member)
my_frr/python/clippy/elf.py:167:30: E1101: Instance of 'ELFDissectData' has no 'elfclass' member (no-member)
my_frr/python/clippy/elf.py:172:22: E1101: Instance of 'ELFDissectData' has no '_data' member (no-member)
my_frr/python/clippy/elf.py:388:31: E1101: Class 'ELFDissectUnion' has no 'members' member (no-member)
my_frr/python/clippy/elf.py:398:66: E1101: Class 'ELFDissectUnion' has no 'members' member (no-member)
my_frr/python/clippy/elf.py:415:20: E1101: Instance of 'ELFSubset' has no 'name' member (no-member)
my_frr/python/clippy/elf.py:429:15: E1101: Instance of 'ELFSubset' has no '_obj' member (no-member)
my_frr/python/clippy/elf.py:435:15: E1101: Instance of 'ELFSubset' has no '_obj' member (no-member)
my_frr/python/clippy/elf.py:444:29: E1101: Instance of 'ELFSubset' has no '_elffile' member (no-member)
my_frr/python/clippy/elf.py:447:30: E1101: Instance of 'ELFSubset' has no '_obj' member (no-member)
my_frr/python/clippy/elf.py:449:19: E1101: Instance of 'ELFSubset' has no '_obj' member (no-member)
my_frr/python/clippy/elf.py:468:34: E1101: Instance of 'ELFSubset' has no 'ptrtype' member (no-member)
my_frr/python/clippy/elf.py:469:29: E1101: Instance of 'ELFSubset' has no 'endian' member (no-member)
my_frr/python/clippy/elf.py:469:43: E1101: Instance of 'ELFSubset' has no 'ptrtype' member (no-member)
my_frr/python/clippy/elf.py:498:15: E1101: Instance of 'ELFSubset' has no '_wrap_data' member (no-member)

-----------------------------------
Your code has been rated at 6.84/10

If you are a new contributor to FRR, please see our contributing guidelines.

After making changes, you do not need to create a new PR. You should perform an amend or interactive rebase followed by a force push.

eqvinox · 2021-04-09T18:03:05Z

Pylint found errors in source files changed by this PR:

@qlyoung erm. I can look at fixing these but that definitely does not belong in this PR. Can we please limit pylint to tests/ for the time being?

qlyoung · 2021-04-09T18:29:06Z

Bot's not blocking the PR, I'd rather leave it turned on

eqvinox · 2021-04-09T18:33:30Z

K.
¯\(ツ)/¯

endian.h supplies be*toh() and htobe*() functions. This fixes the build on musl libc. On other systems it seems endian.h comes in transitively from some other header. (Also, all .c files should have config.h or zebra.h as the first include, even if it works without that it's b0rked and only works due to luck.) Tested-by: Lucian Cristian <lucian.cristian@gmail.com> Signed-off-by: David Lamparter <equinox@diac24.net>

polychaeta

Thanks for your contribution to FRR!

Pylint found errors in source files changed by this PR:

Pylint report for my_frr/python/clippy/elf.py:
************* Module clippy.elf
my_frr/python/clippy/elf.py:146:31: E1101: Instance of 'ELFDissectData' has no '_data' member (no-member)
my_frr/python/clippy/elf.py:146:66: E1101: Instance of 'ELFDissectData' has no '_data' member (no-member)
my_frr/python/clippy/elf.py:162:37: E1101: Instance of 'ELFDissectData' has no 'elfclass' member (no-member)
my_frr/python/clippy/elf.py:167:30: E1101: Instance of 'ELFDissectData' has no 'elfclass' member (no-member)
my_frr/python/clippy/elf.py:172:22: E1101: Instance of 'ELFDissectData' has no '_data' member (no-member)
my_frr/python/clippy/elf.py:388:31: E1101: Class 'ELFDissectUnion' has no 'members' member (no-member)
my_frr/python/clippy/elf.py:398:66: E1101: Class 'ELFDissectUnion' has no 'members' member (no-member)
my_frr/python/clippy/elf.py:415:20: E1101: Instance of 'ELFSubset' has no 'name' member (no-member)
my_frr/python/clippy/elf.py:429:15: E1101: Instance of 'ELFSubset' has no '_obj' member (no-member)
my_frr/python/clippy/elf.py:435:15: E1101: Instance of 'ELFSubset' has no '_obj' member (no-member)
my_frr/python/clippy/elf.py:444:29: E1101: Instance of 'ELFSubset' has no '_elffile' member (no-member)
my_frr/python/clippy/elf.py:447:30: E1101: Instance of 'ELFSubset' has no '_obj' member (no-member)
my_frr/python/clippy/elf.py:449:19: E1101: Instance of 'ELFSubset' has no '_obj' member (no-member)
my_frr/python/clippy/elf.py:468:34: E1101: Instance of 'ELFSubset' has no 'ptrtype' member (no-member)
my_frr/python/clippy/elf.py:469:29: E1101: Instance of 'ELFSubset' has no 'endian' member (no-member)
my_frr/python/clippy/elf.py:469:43: E1101: Instance of 'ELFSubset' has no 'ptrtype' member (no-member)
my_frr/python/clippy/elf.py:498:15: E1101: Instance of 'ELFSubset' has no '_wrap_data' member (no-member)

-----------------------------------
Your code has been rated at 6.84/10

If you are a new contributor to FRR, please see our contributing guidelines.

After making changes, you do not need to create a new PR. You should perform an amend or interactive rebase followed by a force push.

LabN-CI · 2021-04-09T22:49:31Z

💚 Basic BGPD CI results: SUCCESS, 0 tests failed

Results table

_	_
Result	SUCCESS git merge/8421 `0490ce4`
Date	04/09/2021
Start	18:07:25
Finish	18:49:27
Run-Time	42:02
Total	1815
Pass	1815
Fail	0
Valgrind-Errors	0
Valgrind-Loss	0
Details	vncregress-2021-04-09-18:07:25.txt
Log	autoscript-2021-04-09-18:08:39.log.bz2
Memory	445 475 426

For details, please contact louberger

NetDEF-CI · 2021-04-10T16:52:02Z

Continuous Integration Result: SUCCESSFUL

Congratulations, this patch passed basic tests

Tested-by: NetDEF / OpenSourceRouting.org CI System

CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-18300/

This is a comment from an automated CI system.
For questions and feedback in regards to this CI system, please feel free to email
Martin Winter - mwinter (at) opensourcerouting.org.

eqvinox added 2 commits April 8, 2021 00:00

lib: add DT_REL to elf_py/clippy ELF code

8169361

ARM (32-bit) needs DT_REL... and here I was hoping I could avoid the trouble. Fixes: FRRouting#8355 Signed-off-by: David Lamparter <equinox@diac24.net>

python: fix 32-bit pointers in xrelfo container_of

b17f302

This was mistakenly using the host platform's pointer size rather than the ELF file's. Only noticeable when cross compiling... Signed-off-by: David Lamparter <equinox@diac24.net>

polychaeta added clippy libfrr labels Apr 7, 2021

build: don't link pathd/path_main.c twice

636aad9

Can't have things duplicate in libpath.a and pathd directly, they'll crash into eath other on linking. No idea why this doesn't error out in our CI builds, but it definitely breaks LTO builds. Signed-off-by: David Lamparter <equinox@diac24.net>

polychaeta reviewed Apr 9, 2021

View reviewed changes

eqvinox mentioned this pull request Apr 9, 2021

some cross-compile issues #8355

Closed

polychaeta reviewed Apr 9, 2021

View reviewed changes

This comment has been minimized.

Sign in to view

mjstapp approved these changes Apr 12, 2021

View reviewed changes

mjstapp merged commit 53c42c8 into FRRouting:master Apr 12, 2021

eqvinox deleted the xrelfo-arm branch April 18, 2021 07:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix xrelfo on ARM(32) & cross-compile #8421

fix xrelfo on ARM(32) & cross-compile #8421

eqvinox commented Apr 7, 2021

eqvinox commented Apr 7, 2021

NetDEF-CI commented Apr 8, 2021 •

edited

Loading

Continuous Integration Result: SUCCESSFUL

LabN-CI commented Apr 8, 2021 •

edited

Loading

Basic BGPD CI results: SUCCESS, 0 tests failed

lucize commented Apr 8, 2021

eqvinox commented Apr 8, 2021

lucize commented Apr 8, 2021

eqvinox commented Apr 8, 2021

lucize commented Apr 8, 2021 •

edited

Loading

eqvinox commented Apr 8, 2021

eqvinox commented Apr 8, 2021

polychaeta left a comment

eqvinox commented Apr 9, 2021

qlyoung commented Apr 9, 2021

eqvinox commented Apr 9, 2021 •

edited

Loading

polychaeta left a comment

LabN-CI commented Apr 9, 2021

This comment has been minimized.

This comment has been minimized.

NetDEF-CI commented Apr 10, 2021

fix xrelfo on ARM(32) & cross-compile #8421

fix xrelfo on ARM(32) & cross-compile #8421

Conversation

eqvinox commented Apr 7, 2021

eqvinox commented Apr 7, 2021

NetDEF-CI commented Apr 8, 2021 • edited Loading

Continuous Integration Result: SUCCESSFUL

LabN-CI commented Apr 8, 2021 • edited Loading

Basic BGPD CI results: SUCCESS, 0 tests failed

lucize commented Apr 8, 2021

eqvinox commented Apr 8, 2021

lucize commented Apr 8, 2021

eqvinox commented Apr 8, 2021

lucize commented Apr 8, 2021 • edited Loading

eqvinox commented Apr 8, 2021

eqvinox commented Apr 8, 2021

polychaeta left a comment

Choose a reason for hiding this comment

eqvinox commented Apr 9, 2021

qlyoung commented Apr 9, 2021

eqvinox commented Apr 9, 2021 • edited Loading

polychaeta left a comment

Choose a reason for hiding this comment

LabN-CI commented Apr 9, 2021

💚 Basic BGPD CI results: SUCCESS, 0 tests failed

This comment has been minimized.

This comment has been minimized.

NetDEF-CI commented Apr 10, 2021

Continuous Integration Result: SUCCESSFUL

NetDEF-CI commented Apr 8, 2021 •

edited

Loading

LabN-CI commented Apr 8, 2021 •

edited

Loading

lucize commented Apr 8, 2021 •

edited

Loading

eqvinox commented Apr 9, 2021 •

edited

Loading