On Thu, 2 May 2024 08:23:38 GMT, Vladimir Petko <vpe...@openjdk.org> wrote:

> Hi, 
> 
> The     `.type _SafeFetch32_impl,@function` symbol declaration contains a 
> spurious underscore causing an undefined symbol in libjvm.so.
> 
> I am not sure about change in default flags. I have removed the flag that 
> allows linking with undefined symbols
> because having the flag on might cause bugs like this one or 
> https://bugs.openjdk.org/browse/JDK-8329983 as the build succeeds even if 
> some symbols are not defined.
> Openjdk builds successfully without it on amd64, i386, armhf, arm64, s390, 
> ppc64el and riscv64 with this change[1]. riscv64 build was done locally 
> inside vm:
> 
> Finished building target 'images' in configuration 
> 'linux-riscv64-server-release'
> ubuntu@ubuntu:~/jdk$ 
> 
> Unfortunately I do not know why it was introduced, so I might be missing 
> something.
> I am happy to drop it from this one or move it to a separate issue/pr.
>  
> [1] 
> https://launchpad.net/~vpa1977/+archive/ubuntu/october-21/+sourcepub/16076564/+listing-archive-extra

So, after sleeping on this, here is my analysis/verdict:
1) The fix on renaming the `.type` directive looks good. This is the actual bug 
fix.

2) We can remove `--allow-shlib-undefined`. That just overrides 
`--no-allow-shlib-
undefined`, which has been the default for years and years, and there is no 
good reason to have it. It is not strictly related to the bug fix per se, but 
we can do that in this PR as well to keep things simpler.

3) The reason this could happen was since we used assembler code. A similar 
problem in C++ code would not have been allowed by the compiler.

4) If we introduce a common macro as I suggest, we can avoid this ever 
happening again. So while the linker cannot guarantee the consistency when 
linking libjvm.so (which I really would have liked), by using a stricter scheme 
when defining symbols in assembly code, we can make sure to avoid it.

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

Marked as reviewed by ihse (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/19048#pullrequestreview-2045354117

Reply via email to