On Sat, 20 Feb 2021 04:02:37 GMT, Yasumasa Suenaga <[email protected]> wrote:

>> BTW, one way to test these changes might be to do a `clhsdb pmap` and then 
>> do a `findpc` on the last address of each library's map (should find a 
>> symbol) and the first address after the map (should not find a symbol unless 
>> that happens to be the start of the next mapped library). I'm not sure if 
>> this will always work. It's possible that no symbol covers the very end of 
>> the map, especially due to page alignment. Maybe try the first address of 
>> the last page then. I'm not necessarily suggesting you write a test that 
>> does this, but maybe just do a bit of experimenting as a sanity check.
>
>> BTW, one way to test these changes might be to do a `clhsdb pmap` and then 
>> do a `findpc` on the last address of each library's map (should find a 
>> symbol) and the first address after the map (should not find a symbol unless 
>> that happens to be the start of the next mapped library). I'm not sure if 
>> this will always work. It's possible that no symbol covers the very end of 
>> the map, especially due to page alignment. Maybe try the first address of 
>> the last page then.
> 
> I thought similar tests, but I don't want to do so because it might fail as 
> you said.
> As an option, we can add the test to to compare address range of shared 
> libraries between `clhsdb pmap` and `/proc/<PID>/maps`. But I can add it for 
> Linux only, so I'm not sure it is worth to add.

Thank you for the update.

> lib->end is declared as unsigned (uintptr_t), so we can't use (lib->end < 
> aligned_end) when lib->end is set to -1.
Could we use the 0L instead of -1L? What's wrong with 0L?

In general, I'm okay with your fix though.

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

PR: https://git.openjdk.java.net/jdk/pull/2563

Reply via email to