Skip to content

Commit

Permalink
Remove two tests from test_logging_apis.cc (#19100)
Browse files Browse the repository at this point in the history
### Description
In some environments the test code has undefined behavior. To prove it, save the following code as
test.cpp
```c++
#include <iostream>
#include <stdio.h>

int main(){
  char buf[1024];
  int ret = snprintf(buf, sizeof(buf), "%ls","abc");
  if(ret <0){
    std::cout<< ret<< std::endl;
  } else{
    std::cout<< "OK: ret="<<ret<< std::endl;
  }
  return 0;
}
```
Then compile it as 
```
g++   -DNDEBUG -std=gnu++17    test.cpp -o /tmp/t
```
Or 
```
g++   -O2 -DNDEBUG -std=gnu++17    test.cpp -o /tmp/t
```
The first command is without optimization. The second one turns on
optimization. Then the outputs are different.
When optimization is enabled, the output might be:
```
OK: ret=-1
```
You cannot explain why it would go to this branch when ret is "-1". It
might be a bug of a specific version of GCC. However, at this moment we
cannot change the version. It was found in GCC version 8.5.0 20210514
(Red Hat 8.5.0-18) (GCC) that is provided by UBI8. RHEL9 doesn't have
the problem. snprintf is a builtin function of GCC. So the problem was
not related to glibc.

### Motivation and Context
<!-- - Why is this change required? What problem does it solve?
- If it fixes an open issue, please link to the issue here. -->
  • Loading branch information
snnn authored and mszhanyi committed Jan 15, 2024
1 parent ffe4df6 commit 48d163a
Showing 1 changed file with 0 additions and 17 deletions.
17 changes: 0 additions & 17 deletions onnxruntime/test/logging_apis/test_logging_apis.cc
Original file line number Diff line number Diff line change
Expand Up @@ -269,23 +269,6 @@ TEST_F(RealCAPITestsFixture, CppApiORTCXXLOGF) {

line_num = __LINE__ + 1;
ORT_CXX_LOGF_NOEXCEPT(cpp_ort_logger, OrtLoggingLevel::ORT_LOGGING_LEVEL_INFO, "Ignored %d", line_num);

//
// Test errors due to formatting error.
//

// Catch expected exception from ORT_CXX_LOGF macro.
try {
line_num = __LINE__ + 1;
ORT_CXX_LOGF(cpp_ort_logger, OrtLoggingLevel::ORT_LOGGING_LEVEL_ERROR, "%ls", "abc");
FAIL();
} catch (const Ort::Exception& excpt) {
ASSERT_THAT(excpt.what(), testing::HasSubstr("Failed to log message due to formatting error"));
}

// The formatting error is ignored with the ORT_CXX_LOGF_NOEXCEPT macro
line_num = __LINE__ + 1;
ORT_CXX_LOGF_NOEXCEPT(cpp_ort_logger, OrtLoggingLevel::ORT_LOGGING_LEVEL_ERROR, "%ls", "abc");
}

TEST_F(MockCAPITestsFixture, CppLogMacroBypassCApiCall) {
Expand Down

0 comments on commit 48d163a

Please sign in to comment.