On Fri, 29 Sep 2023 07:51:19 GMT, Robbin Ehn <r...@openjdk.org> wrote:

>> Hi please consider.
>> 
>> This works with 2.30, 2.34, 2.38, 2.39, 2.40, 2.41 and current master head. 
>> (tested x64 and some RV)
>> 
>> There are 4 changes in binutils we work around.
>> - zstd compressed debug sections
>> - libsframe added
>> - init_disassemble_info() change
>> - libbfd.a is only present in .lib directory in newer binutils builds (older 
>> it is in both directories) (I think the issue is that we never do make 
>> install, thus have dependency on internal artifact placement)
>> 
>> Specific to RV, there is a bug in binutils causing the standard extensions 
>> not being added to disassembler if we pass in NULL.
>> 
>> This no way near perfect, but at least we can build hsdis with any 
>> contemporary binutils.
>> 
>> Todo better I think we need to build and install binutils to check the 
>> version and then use that version to figure out what options to use when 
>> re-building and re-installing binutils for hsdis.
>> 
>> I asked tool-chain people about our issues, they said, you can't do that. 
>> I.e. have source dependencies on many binutils versions.
>> 
>> As RV is new and have new instructions added to it frequently we really need 
>> to be able to build with bleeding-edge binutils. (capstone RV is not 
>> actively worked on, llvm have many more dependencies)
>
> Robbin Ehn has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Check API during conf

make/autoconf/lib-hsdis.m4 line 266:

> 264:   AC_MSG_CHECKING([Checking binutils API])
> 265:   if test -n "$BINUTILS_SRC"; then
> 266:     include_dis="#include \"$BINUTILS_SRC/include/dis-asm.h\""

All this quoting and duplication seems quite messy. Can't you just set e.g. 
`binutils_include_dir` to either `%BINUTILS_SRC/include` or 
`%BINUTILS_DIR/include`, and then just do 


AC_LANG_PROGRAM([#include "$binutils_include_dir/dis-asm.h"], ...

?

(Code written in github editor; warning for quoting mistakes etc)

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/15138#discussion_r1341201449

Reply via email to