JDK-8343802 : Prevent NULL usage backsliding
  • Type: Enhancement
  • Component: hotspot
  • Sub-Component: test
  • Priority: P4
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2024-11-07
  • Updated: 2025-06-12
  • Resolved: 2025-02-14
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 25
25 b11Fixed
Related Reports
Duplicate :  
JDK-8349477 - Add test to prevent NULL backsliding in hotspot sources and tests
Relates :  
JDK-8342860 - Fix more NULL usage backsliding
Relates :  
JDK-8344917 - Fix recent NULL usage backsliding
Relates :  
JDK-8345647 - Fix recent NULL usage backsliding in Shenandoah
Relates :  
JDK-8343801 - Change string printed by nsk_null_string for null strings
Relates :  
JDK-8324686 - Remove redefinition of NULL for MSVC
Relates :  
JDK-8343800 - Cleanup definition of NULL_WORD
Relates :  
JDK-8346008 - Fix recent NULL usage backsliding in Shenandoah
Sub Tasks
JDK-8349477 :  
Add test to prevent NULL backsliding in hotspot sources and tests - Closed
Description
The HotSpot style guide says nullptr should be used, rather than NULL.  A series of changes were made to eliminate uses of NULL in HotSpot, along with some followups to fix backsliding.  It would be helpful to have the build system prevent the introduction of new uses of NULL.

Currently a sufficient mechanism could be built using
egrep "[^[:alnum:]_]NULL([^[:alnum:]_]|$)"
on {src,test}/hotspot, filtering out some hits, and checking for any residue.
If there are files remaining, report them and fail to build.

In hotspot/src, filter out the following files:
src/hotspot/share/prims/jvmti.xml
src/hotspot/share/prims/jvmti.xsl
These files contain code snippets used to generate C code, not C++.

src/hotspot/share/utilities/globalDefinitions_visCPP.hpp
src/hotspot/share/utilities/globalDefinitions_gcc.hpp
See JDK-8324686 and JDK-8343800.

In hotspot/test, filter out all .c and .java files.  In addition, filter out the following files:

test/hotspot/jtreg/vmTestbase/nsk/share/native/nsk_tools.hpp
See JDK-8343801.

test/hotspot/jtreg/vmTestbase/nsk/share/jvmti/README
test/hotspot/jtreg/vmTestbase/nsk/share/jni/README
test/hotspot/jtreg/vmTestbase/nsk/share/jni/README
These files contain sample C code, not C++.
Comments
Changeset: fa1bd234 Branch: master Author: Nizar Benalla <nbenalla@openjdk.org> Date: 2025-02-14 12:24:36 +0000 URL: https://git.openjdk.org/jdk/commit/fa1bd2344e60163bf247c668b94f98c50c72855a
14-02-2025
A pull request was submitted for review. Branch: master URL: https://git.openjdk.org/jdk/pull/23466 Date: 2025-02-05 15:39:32 +0000
05-02-2025
Assigning this to myself to avoid any duplicate work. I'll try to set this up to work locally and on our CI.
05-12-2024
I asked [~nbenalla] to take a look at this if he can implement a Java test grepping the hotspot sources for NULL.
05-12-2024
If we are doing this as a jtreg test, it needs to be Java and not a shell script. Opening a file and streaming lines is quite straightforward however.
04-12-2024
So maybe creating a `test/hotspot/jtreg/src` directory, and put a no-NULL-in-hotspot-src test there. Are adding new JTREG shellscript tests completely verboten, or would it be acceptable in a case like this? Doing this as a shellscript is basically a one-liner; doing it in Java is not impossible but slightly more work. (How much depends on if we want to allow Java code to call `grep`, or if it needs to reimplement this using java regex classes.)
04-12-2024
We have the full source available when jtreg tests are run in our internal systems. The same should be true in GHA, as well as when developers run tests locally.
04-12-2024
Having it be a tier1 test requires the test to have access to the source tree. Does that access already exist? It seems like this could perhaps be modeled on the (closed) validate-headers make target? I took a brief look at that and my eyes crossed; it's not that much code, but it is more complicated than I had expected. But I guess that's make for you.
04-12-2024
If this was to be implemented in jcheck, then I would imagine it being a more generalized regex check, where .jcheck/conf would have to specify what regex to check for and file globs to apply the check to. Still don't think it's the right thing to do. I think we would rather want a more direct feedback at build time rather than when creating a PR.
02-12-2024
jcheck is maintained outside the code repository, so I think that is a worse solution. It does not, and should not, know about stuff like "hotspot code", so any changes to jcheck will need to apply to all native code, or we'd have to add functionality to filter some jchecks on paths, etc. A lot of extra work for no good reason. So if this should be implemented, it should be implemented in the JDK repo. I've been thinking before of having a "RunCheck" system, similar to "RunTest", but that does checks on e.g. the code base; where checks might be less of a clear fail/succeed dichotomy, but might involve pointing out stuff that might need investigation. Basically, be more tolerant to false positives. I have a prototype for this in a personal branch but it's been stale for some months. But maybe the NULL check is so strict that it should be a real, honest-to-god, test that must not fail, part of tier1?
02-12-2024
I disagree that clang-tidy or some compiler option or the like is better than using grep for this. The prior work on removing uses of NULL was driven via grep searches. It intentionally dealt with non-identifier uses as well, e.g. uses in comments and strings. In part that's to prevent comments containing what looks like code (e.g. NULL identifiers) from differing from the actual code. In part that to makes it easier to spot identifier uses when using grep. So we're well set up for using grep for this purpose. And using a compiler-like tool will miss cases that we think we want to prevent, such as NULL in comments. It is also limited to the platforms and ports that the tool supports, while a grep-based approach deals with the entire code base. So I continue to believe a grep-based approach to this is the way to go. I think the only interesting question is whether it's part of the build or part of jcheck. I'd think whichever is easier to implement and maintain, but I don't know which one that would be.
27-11-2024
Ah, sorry, you said `clang-tidy`, not `clang`. Yeah, then that is more work. But maybe worth it nevertheless. I am all in favor of making it easier to run static analysis tools on the JDK.
26-11-2024
That sounds much better! The compiler has the semantic knowledge to determine this properly; I'm wary of running general regexps on the source code...
26-11-2024
clang-tidy has modernize-use-nullptr: "The check converts the usage of null pointer constants (e.g. NULL, 0) to use the new C++11 and C23 nullptr keyword." Perhaps something could be built on that? Doing so is likely more work than the suggested grep-based approach.
26-11-2024