8367609: serviceability/sa/ClhsdbPmap.java fails when built with Clang #27274

fandreuz · 2025-09-14T23:16:00Z

The problem seems to be in read_lib_segments (ps_core.c), this check is too harsh:

jdk/src/jdk.hotspot.agent/linux/native/libsaproc/ps_core.c

Lines 423 to 425 in 5271448

    
           if ((existing_map->memsz != page_size) && 
        
               (existing_map->fd != lib_fd) && 
        
               (ROUNDUP(existing_map->memsz, page_size) != ROUNDUP(lib_php->p_memsz, page_size))) {

In my run, existing_map->memsz = 0xe24000, while the rhs in L425 is 0xe23000. According to the NT_FILE entry, this segment of libjvm.so has file offset 0x67f000. It seems that the linker aligned it down according to the page size (0x1000). The offset of the same segment according to readelf -l libjvm.so is 0x67fc80. This additional offset should be added to p_memsz to obtain the 0xe24000, which we see in the core dump.

I added some files to the ticket for context.

Passes tier1 and tier2.

Progress

Change must be properly reviewed (1 review required, with at least 1 Reviewer)
Change must not contain extraneous whitespace
Commit message must refer to an issue

Issue

JDK-8367609: serviceability/sa/ClhsdbPmap.java fails when built with Clang (Bug - P4)

Reviewers

Kevin Walls (@kevinjwalls - Reviewer)
Chris Plummer (@plummercj - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/27274/head:pull/27274
$ git checkout pull/27274

Update a local copy of the PR:
$ git checkout pull/27274
$ git pull https://git.openjdk.org/jdk.git pull/27274/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 27274

View PR using the GUI difftool:
$ git pr show -t 27274

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/27274.diff

Using Webrev

Link to Webrev Comment

bridgekeeper · 2025-09-14T23:17:48Z

👋 Welcome back fandreuzzi! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2025-09-14T23:17:54Z

@fandreuz This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8367609: serviceability/sa/ClhsdbPmap.java fails when built with Clang

Reviewed-by: kevinw, cjplummer

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 49 new commits pushed to the master branch:

db6320d: 8368968: FloatingDecimal: Clean up unused code
ef724f4: 8368985: Small Float16 refactorings
6b72b77: 6177299: [Fmt-Nu] NumberFormat.getPercentInstance() does not work correctly
... and 46 more: https://git.openjdk.org/jdk/compare/320230db5f9ca95f23218704cb2e69521e03852f...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

As you do not have Committer status in this project an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@kevinjwalls, @plummercj) but any other Committer may sponsor as well.

➡️ To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).

openjdk · 2025-09-14T23:18:33Z

@fandreuz The following label will be automatically applied to this pull request:

serviceability

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

mlbridge · 2025-09-14T23:23:26Z

Webrevs

src/jdk.hotspot.agent/linux/native/libsaproc/ps_core.c

kevinjwalls · 2025-09-17T11:48:03Z

Hi - Do you still have the same core? Can you attach a "readelf -a " output from it in the jbs issue with the other (helpful) files? Would like to see how things get mapped.

It's interesting to me at the moment that in a random build of mine with gcc, libjvm has e.g.

                 FileSiz            MemSiz              Flags  Align
...
  LOAD           0x0000000000000000 0x0000000000000000 0x0000000000000000
                 0x00000000022a5a50 0x00000000022a5a50  R E    0x1000

..so loadable text is just part of the mapping at the base address, no offset.

A core of that contains:

  LOAD           0x000000003431b000 0x00007f1305a15000 0x0000000000000000
                 0x00000000022a6000 0x00000000022a6000  R E    0x1000

But your clang build has the text with some offset and vaddr:
https://bugs.openjdk.org/secure/attachment/116147/libjvm_sections.txt

Program Headers:
  Type           Offset             VirtAddr           PhysAddr
                 FileSiz            MemSiz              Flags  Align
...
  LOAD           0x000000000067fc80 0x0000000000680c80 0x0000000000680c80
                 0x0000000000e225c0 0x0000000000e225c0  R E    0x1000

..so does that appear as a distinct PH in the core?

fandreuz · 2025-09-17T19:58:08Z

Hi @kevinjwalls, I don't have the same core. I ran the test another time, you find the new files attached to the ticket (those starting with 2).

This is the corresponding entry in the new core:

  LOAD           0x0000000006b23000 0x00007fa9ff881000 0x0000000000000000
                 0x0000000000e23000 0x0000000000e23000  R E    0x1000

kevinjwalls · 2025-09-18T10:26:13Z

Thanks - does that still have the same problem?
(Do you have the jtreg log from this one, to confirm I was looking at the right address here...)

libjvm second seg:
core:

Type           Offset             VirtAddr           PhysAddr
                 FileSiz            MemSiz              Flags  Align
  LOAD           0x0000000006b23000 0x00007fa9ff881000 0x0000000000000000
                 0x0000000000e23000 0x0000000000e23000  R E    0x1000

libjvm binary:

LOAD           0x0000000000680180 0x0000000000681180 0x0000000000681180
               0x0000000000e226c0 0x0000000000e226c0  R E    0x1000

These figures look like they work, for that check which has been working for gcc builds:

ROUNDUP(existing_map->memsz, page_size) != ROUNDUP(lib_php->p_memsz, page_size)

left: core file memsize: 0x0000000000e23000
right: lib mem size 0x0000000000e226c0 rounds to 0xe23000

Was core 1 different? Looks like a slightly smaller libjvm in that run (mem size was 0e225c0 and the core contained a 0xe24000 size mapping, which was the problem).

fandreuz · 2025-09-18T21:07:39Z

Hi @kevinjwalls, I think that might be my fault. I rerun the test and I noticed the failure this time is not in libjvm.so, but in libjimage.so:

hsdb> ERROR: address conflict @ 0x7fa9ff0e4440 (existing map size = 102400, size = 97328, flags = 5)

% info proc mappings
[...]
0x00007fa9ff0e4000 0x00007fa9ff0fd000 0x19000            0x8000             /home/fandreuz/code/jdk/build/clang/images/jdk/lib/libjimage.so

I think this happens by chance. I added the files from the new round to the ticket.

kevinjwalls · 2025-09-19T10:25:00Z

failure this time is not in libjvm.so, but in libjimage.so:

Thanks looking again - do you still have the files, as libjimage_sections.txt ... has the Section Headers, not the Program Headers (readelf -a is fine 8-) )

fandreuz · 2025-09-19T20:53:20Z

Hi @kevinjwalls, I just uploaded 3libjimage_all.txt, thanks again for looking into this PR

kevinjwalls · 2025-09-23T14:24:47Z

Thanks for the additional info.

If I ignore what's said so far and start again, I see the following...  (anyone should feel free to correct, we aren't in this
area every day!...)

hsdb> ERROR: address conflict @ 0x7fa9ff0e4440 (existing map size = 102400, size = 97328, flags = 5)

          print_error("address conflict @ 0x%lx (existing map size = %ld, size = %ld, flags = %d)\n",
                        target_vaddr, existing_map->memsz, lib_php->p_memsz, lib_php->p_flags);

core has no mapping at exactly 0x7fa9ff0e4440 but has:

  Start                 End               Page Offset
  0x00007fa9ff0db000  0x00007fa9ff0e4000  0x0000000000000000 /home/fandreuz/code/jdk/build/clang/images/jdk/lib/libjimage.so  (0x9000 size) 0x841c rounded up
  0x00007fa9ff0e4000  0x00007fa9ff0fd000  0x0000000000000008 /home/fandreuz/code/jdk/build/clang/images/jdk/lib/libjimage.so (0x19000 size) the existing map size from the error
  

The error is having a size 0x17c30 mapping that should go at 0x00007fa9ff0e4000
That is the second LOAD phdr from libjimage.

The check which has been working for gcc builds:
ROUNDUP(existing_map->memsz, page_size) != ROUNDUP(lib_php->p_memsz, page_size)

102400 	= 0x19000 is mem size, page size aligned. The core has a mapping of this size.
97328	= 0x17c30 libjimage has this, which rounds up to only 0x18000

libjimage is providing too little data? 
But target vaddr 0x7fa9ff0e4440 is offset into the actual segment 0x00007fa9ff0e4000 by  0x440  (1088 bytes)

0x17c30 + 0x440 = 0x18070 which rounds up to the wanted 0x19000


src/jdk.hotspot.agent/linux/native/libsaproc/ps_core.c

399       uintptr_t target_vaddr = lib_php->p_vaddr + lib_base;
400       map_info *existing_map = core_lookup(ph, target_vaddr);

(Existing code expects target_vaddr and existing_map->vaddr to be exactly equal, not to see target_vaddr being
anything other than 0x1000 aligned.)

Maybe we:
check if target_vaddr > existing_map->vaddr and add any difference to the library mem size we compare?


429             (ROUNDUP(existing_map->memsz, page_size) != ROUNDUP(lib_php->p_memsz, page_size))) {

+        uint64_t lib_memsz = lib_php->p_memsz; // check type
+        if (target_vaddr > existing_map->vaddr) {
+            lib_memsz += (target_vaddr - existing_map->vaddr);
+        }
+        lib_memsz = ROUNDUP(lib_memsz, page_size);
+
         if ((existing_map->memsz != page_size) &&
             (existing_map->fd != lib_fd) &&
-            (ROUNDUP(existing_map->memsz, page_size) != ROUNDUP(lib_php->p_memsz, page_size))) {
+            (ROUNDUP(existing_map->memsz, page_size) != lib_memsz)) {


This is kind of similar to what you had, but maybe the target_vaddr varies and becomes offset even more
than pagesize at some point?

Curious if that works for clang builds.  In my builds it doesn't change anything, we don't hit
target_vaddr > existing_map->vaddr

fandreuz · 2025-09-28T21:16:16Z

Hi @kevinjwalls, what you say makes sense to me. I tried the updated check on my environment and it fixed the problem as well as what I initially proposed in this PR.

I updated the PR too, the new check looks cleaner than what I had.

kevinjwalls

OK glad it works. 8-)

Maybe a comment above line 423 so say something like:
// Account for the PH being at some vaddr offset from mapping in core file.

We could have left the ROUNDUP on line 427 to the comparison on line 431 to make it more like the original. I think either way is good.

Good for me, let's check Chris who looked at it also agrees.

plummercj · 2025-09-29T20:49:41Z

src/jdk.hotspot.agent/linux/native/libsaproc/ps_core.c

+        // Account for the PH being at some vaddr offset from mapping in core file.
+        uint64_t lib_memsz = lib_php->p_memsz;
+        if (target_vaddr > existing_map->vaddr) {
+            lib_memsz += target_vaddr - existing_map->vaddr;
+        }
+


I think this change looks good, but just want to make sure I'm understanding it properly. Kevin commented the following:

hsdb> ERROR: address conflict @ 0x7fa9ff0e4440 (existing map size = 102400, size = 97328, flags = 5) print_error("address conflict @ 0x%lx (existing map size = %ld, size = %ld, flags = %d)\n", target_vaddr, existing_map->memsz, lib_php->p_memsz, lib_php->p_flags); core has no mapping at exactly 0x7fa9ff0e4440 but has: Start End Page Offset 0x00007fa9ff0db000 0x00007fa9ff0e4000 0x0000000000000000 /home/fandreuz/code/jdk/build/clang/images/jdk/lib/libjimage.so (0x9000 size) 0x841c rounded up 0x00007fa9ff0e4000 0x00007fa9ff0fd000 0x0000000000000008 /home/fandreuz/code/jdk/build/clang/images/jdk/lib/libjimage.so (0x19000 size) the existing map size from the error

So the problem here is that we were expecting 0x00007fa9ff0e4000 but got 0x7fa9ff0e4440. Basically target_vaddr is at an unexpected offset from existing_map->vaddr. The fix is to ad this offset to lib_memsz so the error is no longer triggered. Do we understand why this offset is happening?

Do we understand why this offset is happening?

It looks like the binary is just built differently, clang or the linker or some combination arranges things differently to what we have seen from gcc.

Maybe it's enough to say there doesn't have to be a 1:1 mapping from program headers to segments in the process.
We can have a core file segment that contains more than one program header from the library (that 0x19000 size segment at the libjimage base address).

So, new question:

we must have been through this loop already, in the libjimage example with the first LOAD PH of size 0x000000000000841c at the library base address.

Did that pass the same test?
No it can't, the mapping is 0x19000 and size 0x841c does not round up to that.
I think we must ignore the first LOAD PH from the clang build as we check:

408 } else if (lib_php->p_flags != existing_map->flags) {

..and the first LOAD PH from https://bugs.openjdk.org/secure/attachment/116209/3libjimage_all.txt
..is a Read only, not execute.

This should be OK in this case:

https://bugs.openjdk.org/secure/attachment/116199/3core_all.txt
Type Offset VirtAddr PhysAddr
FileSiz MemSiz Flags Align
LOAD 0x0000000000141000 0x00007fa9ff0e4000 0x0000000000000000
0x0000000000019000 0x0000000000019000 R E 0x1000

Filesize and memsiz both 0x019000 must mean the core contains everything, we don't need either of these from the library.
But there is no harm in continuing past the problematic check and populating existing_map from the library.

Ok. That's mostly making sense to me. I'm still a bit fuzzy on it, but that's ok. Has testing been done to make sure this doesn't break anything with gcc?

Has testing been done to make sure this doesn't break anything with gcc?

Yes, I didn't see any problem while testing the change with gcc.

plummercj

Looks good.

fandreuz · 2025-10-01T22:37:07Z

Thanks for the review @kevinjwalls and @plummercj !

fandreuz · 2025-10-01T22:37:13Z

/integrate

openjdk · 2025-10-01T22:38:10Z

@fandreuz
Your change (at version fe9adaf) is now ready to be sponsored by a Committer.

kevinjwalls · 2025-10-02T11:39:55Z

/sponsor

openjdk · 2025-10-02T11:41:33Z

Going to push as commit 8be1616.
Since your change was applied there have been 53 commits pushed to the master branch:

dfd3832: 8368966: Remove spurious VMStructs friends
5251405: 8368848: JShell's code completion not always working for multi-snippet inputs
fa3af82: 8338197: [ubsan] ad_x86.hpp:6417:11: runtime error: shift exponent 100 is too large for 32-bit type 'unsigned int'
... and 50 more: https://git.openjdk.org/jdk/compare/320230db5f9ca95f23218704cb2e69521e03852f...master

Your commit was automatically rebased without conflicts.

openjdk · 2025-10-02T11:41:43Z

@kevinjwalls @fandreuz Pushed as commit 8be1616.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

fandreuz added 4 commits September 14, 2025 22:16

fix

2083628

fix

a4d22ab

simple fix

8f2f9bd

fix

0c050d3

openjdk bot added serviceability serviceability-dev@openjdk.org rfr Pull request is ready for review labels Sep 14, 2025

simplify

795338e

plummercj reviewed Sep 15, 2025

View reviewed changes

src/jdk.hotspot.agent/linux/native/libsaproc/ps_core.c Outdated Show resolved Hide resolved

fandreuz added 4 commits September 15, 2025 23:29

review

5b59408

invert

b839799

comment and rename

6943280

nn

0a5ef01

fandreuz added 2 commits September 28, 2025 23:12

Merge branch 'master' into JDK-8367609

58a9dd0

new check

70eaa2b

kevinjwalls approved these changes Sep 29, 2025

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Sep 29, 2025

cc

fe9adaf

openjdk bot removed the ready Pull request is ready to be integrated label Sep 29, 2025

kevinjwalls approved these changes Sep 29, 2025

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Sep 29, 2025

plummercj reviewed Sep 29, 2025

View reviewed changes

plummercj approved these changes Oct 1, 2025

View reviewed changes

openjdk bot added the sponsor Pull request is ready to be sponsored label Oct 1, 2025

openjdk bot added the integrated Pull request has been integrated label Oct 2, 2025

openjdk bot closed this Oct 2, 2025

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review sponsor Pull request is ready to be sponsored labels Oct 2, 2025

	if ((existing_map->memsz != page_size) &&
	(existing_map->fd != lib_fd) &&
	(ROUNDUP(existing_map->memsz, page_size) != ROUNDUP(lib_php->p_memsz, page_size))) {

8367609: serviceability/sa/ClhsdbPmap.java fails when built with Clang #27274

8367609: serviceability/sa/ClhsdbPmap.java fails when built with Clang #27274

Uh oh!

Conversation

fandreuz commented Sep 14, 2025 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issue

Reviewers

Reviewing

Uh oh!

bridgekeeper bot commented Sep 14, 2025

Uh oh!

openjdk bot commented Sep 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openjdk bot commented Sep 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mlbridge bot commented Sep 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Webrevs

Uh oh!

Uh oh!

kevinjwalls commented Sep 17, 2025

Uh oh!

fandreuz commented Sep 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kevinjwalls commented Sep 18, 2025

Uh oh!

fandreuz commented Sep 18, 2025

Uh oh!

kevinjwalls commented Sep 19, 2025

Uh oh!

fandreuz commented Sep 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kevinjwalls commented Sep 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fandreuz commented Sep 28, 2025

Uh oh!

kevinjwalls left a comment

Choose a reason for hiding this comment

Uh oh!

plummercj Sep 29, 2025

Choose a reason for hiding this comment

Uh oh!

kevinjwalls Sep 30, 2025

Choose a reason for hiding this comment

Uh oh!

plummercj Oct 1, 2025

Choose a reason for hiding this comment

Uh oh!

fandreuz Oct 1, 2025

Choose a reason for hiding this comment

Uh oh!

plummercj left a comment

Choose a reason for hiding this comment

Uh oh!

fandreuz commented Oct 1, 2025

Uh oh!

fandreuz commented Oct 1, 2025

Uh oh!

openjdk bot commented Oct 1, 2025

Uh oh!

kevinjwalls commented Oct 2, 2025

Uh oh!

openjdk bot commented Oct 2, 2025

Uh oh!

openjdk bot commented Oct 2, 2025

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

fandreuz commented Sep 14, 2025 •

edited by openjdk bot

Loading

openjdk bot commented Sep 14, 2025 •

edited

Loading

openjdk bot commented Sep 14, 2025 •

edited

Loading

mlbridge bot commented Sep 14, 2025 •

edited

Loading

fandreuz commented Sep 17, 2025 •

edited

Loading

fandreuz commented Sep 19, 2025 •

edited

Loading

kevinjwalls commented Sep 23, 2025 •

edited

Loading