rmuir commented on issue #11910:
URL: https://github.com/apache/lucene/issues/11910#issuecomment-1312078209
OK i checked out old git hash before #11905 commit, seems like
"NarrowCalculation" is the best one? I think i made a mistake trying to turn on
too many checks at once.
I enabled the check like this:
```
diff --git a/gradle/validation/error-prone.gradle
b/gradle/validation/error-prone.gradle
index 962e237cc03..587c0611ffe 100644
--- a/gradle/validation/error-prone.gradle
+++ b/gradle/validation/error-prone.gradle
@@ -68,6 +68,7 @@ allprojects { prj ->
options.errorprone.disableWarningsInGeneratedCode = true
options.errorprone.errorproneArgs = [
+ '-XepAllErrorsAsWarnings',
'-Xep:InlineMeSuggester:OFF', // We don't use this annotation
// test
@@ -142,7 +143,6 @@ allprojects { prj ->
'-Xep:ModifiedButNotUsed:OFF',
'-Xep:MutablePublicArray:OFF',
'-Xep:NarrowingCompoundAssignment:OFF',
- '-Xep:NarrowCalculation:OFF',
'-Xep:NonAtomicVolatileUpdate:OFF',
'-Xep:NonCanonicalType:OFF',
'-Xep:ObjectToString:OFF',
```
And I run checker with `./gradlew assemble -Pvalidation.errorprone=true
-Pjavac.failOnWarnings=false > ~/overflow.log 2>&1`
Here's what this check said for the old buggy code with overflow, it seems
to find it?:
```
/home/rmuir/workspace/lucene/lucene/core/src/java/org/apache/lucene/codecs/lucene94/Lucene94HnswVectorsReader.java:402:
warning: [NarrowCalculation] This product of integers could overflow before
being implicitly
cast to a long.
graphOffsetsByLevel[level] = (1 + (M * 2)) * Integer.BYTES *
numNodesOnLevel0;
^
(see https://errorprone.info/bugpattern/NarrowCalculation)
Did you mean 'graphOffsetsByLevel[level] = (1 + (M * 2)) * Integer.BYTES *
((long) numNodesOnLevel0);'?
/home/rmuir/workspace/lucene/lucene/core/src/java/org/apache/lucene/codecs/lucene94/Lucene94HnswVectorsReader.java:406:
warning: [NarrowCalculation] This product of integers could overflow before
being implicitly
cast to a long.
graphOffsetsByLevel[level - 1] + (1 + M) * Integer.BYTES *
numNodesOnPrevLevel;
^
(see https://errorprone.info/bugpattern/NarrowCalculation)
Did you mean 'graphOffsetsByLevel[level - 1] + (1 + M) * Integer.BYTES *
((long) numNodesOnPrevLevel);'?
/home/rmuir/workspace/lucene/lucene/core/src/java/org/apache/lucene/codecs/lucene94/Lucene94HnswVectorsReader.java:440:
warning: [NarrowCalculation] This product of integers could overflow before
being implicitly cast to a long.
this.bytesForConns0 = ((long) (entry.M * 2) + 1) * Integer.BYTES;
^
(see https://errorprone.info/bugpattern/NarrowCalculation)
Did you mean 'this.bytesForConns0 = ((long) (entry.M * 2L) + 1) *
Integer.BYTES;'?
```
Here's the log file against current main branch.
[overflow.log](https://github.com/apache/lucene/files/9992734/overflow.log)
This is about as narrow as I can get, as far as a check that would find the
previous bug, to look for similar bugs and reduce the noise. I think this check
looks worth enabling, we can just add some `(long)` casts to the rambytesUsed
and change some `x / 1024 / 1024.` to be `x / 1024. / 1024.` in the merge
policy stuff to address those cases.
Turning on other checks such as narrow-compound-assignment would be good and
reveals even more interesting stuff but its a separate amount of work.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
