Skip to content

8332825: ubsan: guardedMemory.cpp:35:11: runtime error: null pointer passed as argument 2, which is declared to never be null #2829

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

8332825: ubsan: guardedMemory.cpp:35:11: runtime error: null pointer passed as argument 2, which is declared to never be null #2829

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion src/hotspot/share/memory/guardedMemory.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,9 @@ void* GuardedMemory::wrap_copy(const void* ptr, const size_t len, const void* ta
if (outerp != NULL) {
GuardedMemory guarded(outerp, len, tag);
void* innerp = guarded.get_user_ptr();
memcpy(innerp, ptr, len);
if (ptr != nullptr) {
memcpy(innerp, ptr, len);
}
Copy link
Member

@tstuefe tstuefe Aug 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm. innerp can never be null. If anything, we should assert.

Copy link
Member

@GoeLin GoeLin Sep 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi,
I don't understand. First, innerp is not checked for null. Second: does the code in 17 differ to 21&head? Else we should change this to an assertion in head, first.

Copy link
Member

@tstuefe tstuefe Sep 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Goetz!

innerp is the result of

  u_char* get_user_ptr() const {
    assert(_base_addr != nullptr, "Not wrapping any memory");
    return _base_addr + sizeof(GuardHeader);
  }

which cannot return null. _base_addr is the result of a malloc and we only ever enter this path if that malloc succeeded. It is also asserted at least twice.

Cheers, Thomas

Copy link
Member

@GoeLin GoeLin Sep 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but the new code checks whether "ptr" is null. That is passed in as an argument.

Copy link
Member Author

@MBaesken MBaesken Sep 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we call
GuardedMemory::wrap_copy(no_data, 0);
This makes ptr == NULL / nullptr .
See the full backtrace here openjdk/jdk#19382
It is a clean backport so should be the same in 17. The check was added to handle to no_data / length 0 case .

Copy link
Member

@tstuefe tstuefe Sep 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I just misread the code. Sorry for the confusion, I was too tired.

return innerp;
}
return NULL; // OOM
Expand Down