Skip to content
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

Jump to bottom

[C++][CI] Valgrind errors in StringConversion.ToHalfFloat #41154

Closed
pitrou opened this issue Apr 11, 2024 · 2 comments
Closed

[C++][CI] Valgrind errors in StringConversion.ToHalfFloat #41154

pitrou opened this issue Apr 11, 2024 · 2 comments
Assignees
@pitrou
Labels
Component: C++ Component: Continuous Integration Type: bug
Milestone
16.0.0

Comments

@pitrou
Copy link
Member

pitrou commented Apr 11, 2024

Describe the bug, including details regarding any error messages, version, and platform.

See https://dev.azure.com/ursacomputing/crossbow/_build/results?buildId=63003&view=logs&j=0da5d1d9-276d-5173-c4c4-9d4d4ed14fdb&t=d9b15392-e4ce-5e4c-0c8c-b69645229181&l=4381

[ RUN      ] StringConversion.ToHalfFloat
==12283== Conditional jump or move depends on uninitialised value(s)
==12283==    at 0x5CA5C9A: arrow::util::(anonymous namespace)::BinaryConverter<unsigned int>::ToBinary16(unsigned int) (float16.cc:88)
==12283==    by 0x5CA5EDA: arrow::util::Float16::FromFloat(float) (float16.cc:210)
   fun:_ZN7testing4Test3RunEv
   fun:_ZN7testing8TestInfo3RunEv
   fun:_ZN7testing9TestSuite3RunEv
   fun:_ZN7testing8internal12UnitTestImpl11RunAllTestsEv
   fun:_ZN7testing8internal38HandleSehExceptionsInMethodIfSupportedINS0_12UnitTestImplEbEET0_PT_MS4_FS3_vEPKc
   fun:_ZN7testing8internal35HandleExceptionsInMethodIfSupportedINS0_12UnitTestImplEbEET0_PT_MS4_FS3_vEPKc
   fun:_ZN7testing8UnitTest3RunEv
   fun:_Z13RUN_ALL_TESTSv
   fun:main
}
[       OK ] StringConversion.ToHalfFloat (18 ms)

Component(s)

C++, Continuous Integration
@pitrou pitrou self-assigned this Apr 11, 2024
@pitrou pitrou added this to the 16.0.0 milestone Apr 11, 2024
pitrou added a commit to pitrou/arrow that referenced this issue Apr 11, 2024
@pitrou
apacheGH-41154: [C++] Fix Valgrind error in string-to-float16 conversion
14d89bf 
Only do the final conversion to float16 on success, to avoid conditional jump on uninitialized values.
@github-actions github-actions bot mentioned this issue Apr 11, 2024
Merged
pitrou added a commit that referenced this issue Apr 11, 2024
@pitrou
GH-41154: [C++] Fix Valgrind error in string-to-float16 conversion (#…
daa2efa 
…41155)

### Rationale for this change

Only do the final conversion to float16 on success, to avoid conditional jump on uninitialized values.

Note: this is a benign error. But we want our Valgrind CI job to be as successful as possible.

### Are these changes tested?

Yes, on CI.

### Are there any user-facing changes?

No.
* GitHub Issue: #41154

Authored-by: Antoine Pitrou <antoine@python.org>
Signed-off-by: Antoine Pitrou <antoine@python.org>
@pitrou
Copy link
Member Author

pitrou commented Apr 11, 2024

Issue resolved by pull request 41155
#41155

@pitrou pitrou closed this as completed Apr 11, 2024
@pitrou
Copy link
Member Author

pitrou commented Apr 11, 2024

@raulcd This could be cherry-picked in 16.0.0

raulcd pushed a commit that referenced this issue Apr 12, 2024
@pitrou @raulcd
GH-41154: [C++] Fix Valgrind error in string-to-float16 conversion (#…
0f33954 
…41155)

### Rationale for this change

Only do the final conversion to float16 on success, to avoid conditional jump on uninitialized values.

Note: this is a benign error. But we want our Valgrind CI job to be as successful as possible.

### Are these changes tested?

Yes, on CI.

### Are there any user-facing changes?

No.
* GitHub Issue: #41154

Authored-by: Antoine Pitrou <antoine@python.org>
Signed-off-by: Antoine Pitrou <antoine@python.org>
vibhatha pushed a commit to vibhatha/arrow that referenced this issue Apr 15, 2024
@pitrou @vibhatha
apacheGH-41154: [C++] Fix Valgrind error in string-to-float16 convers…
fe777df 
…ion (apache#41155)

### Rationale for this change

Only do the final conversion to float16 on success, to avoid conditional jump on uninitialized values.

Note: this is a benign error. But we want our Valgrind CI job to be as successful as possible.

### Are these changes tested?

Yes, on CI.

### Are there any user-facing changes?

No.
* GitHub Issue: apache#41154

Authored-by: Antoine Pitrou <antoine@python.org>
Signed-off-by: Antoine Pitrou <antoine@python.org>
tolleybot pushed a commit to tmct/arrow that referenced this issue May 2, 2024
@pitrou @tolleybot
apacheGH-41154: [C++] Fix Valgrind error in string-to-float16 convers…
80b3787 
…ion (apache#41155)

### Rationale for this change

Only do the final conversion to float16 on success, to avoid conditional jump on uninitialized values.

Note: this is a benign error. But we want our Valgrind CI job to be as successful as possible.

### Are these changes tested?

Yes, on CI.

### Are there any user-facing changes?

No.
* GitHub Issue: apache#41154

Authored-by: Antoine Pitrou <antoine@python.org>
Signed-off-by: Antoine Pitrou <antoine@python.org>
tolleybot pushed a commit to tmct/arrow that referenced this issue May 4, 2024
@pitrou @tolleybot
apacheGH-41154: [C++] Fix Valgrind error in string-to-float16 convers…
d3e44f6 
…ion (apache#41155)

### Rationale for this change

Only do the final conversion to float16 on success, to avoid conditional jump on uninitialized values.

Note: this is a benign error. But we want our Valgrind CI job to be as successful as possible.

### Are these changes tested?

Yes, on CI.

### Are there any user-facing changes?

No.
* GitHub Issue: apache#41154

Authored-by: Antoine Pitrou <antoine@python.org>
Signed-off-by: Antoine Pitrou <antoine@python.org>
rok pushed a commit to tmct/arrow that referenced this issue May 8, 2024
@pitrou @rok
apacheGH-41154: [C++] Fix Valgrind error in string-to-float16 convers…
c93860f 
…ion (apache#41155)

### Rationale for this change

Only do the final conversion to float16 on success, to avoid conditional jump on uninitialized values.

Note: this is a benign error. But we want our Valgrind CI job to be as successful as possible.

### Are these changes tested?

Yes, on CI.

### Are there any user-facing changes?

No.
* GitHub Issue: apache#41154

Authored-by: Antoine Pitrou <antoine@python.org>
Signed-off-by: Antoine Pitrou <antoine@python.org>
rok pushed a commit to tmct/arrow that referenced this issue May 8, 2024
@pitrou @rok
apacheGH-41154: [C++] Fix Valgrind error in string-to-float16 convers…
a19416c 
…ion (apache#41155)

### Rationale for this change

Only do the final conversion to float16 on success, to avoid conditional jump on uninitialized values.

Note: this is a benign error. But we want our Valgrind CI job to be as successful as possible.

### Are these changes tested?

Yes, on CI.

### Are there any user-facing changes?

No.
* GitHub Issue: apache#41154

Authored-by: Antoine Pitrou <antoine@python.org>
Signed-off-by: Antoine Pitrou <antoine@python.org>
vibhatha pushed a commit to vibhatha/arrow that referenced this issue May 25, 2024
@pitrou @vibhatha
apacheGH-41154: [C++] Fix Valgrind error in string-to-float16 convers…
5f4a773 
…ion (apache#41155)

### Rationale for this change

Only do the final conversion to float16 on success, to avoid conditional jump on uninitialized values.

Note: this is a benign error. But we want our Valgrind CI job to be as successful as possible.

### Are these changes tested?

Yes, on CI.

### Are there any user-facing changes?

No.
* GitHub Issue: apache#41154

Authored-by: Antoine Pitrou <antoine@python.org>
Signed-off-by: Antoine Pitrou <antoine@python.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@pitrou pitrou

Labels
Component: C++ Component: Continuous Integration Type: bug
Projects
None yet
Milestone
16.0.0
Development

No branches or pull requests
1 participant
@pitrou