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.

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

Additionnal issues with sanitizer #18

Closed
Jean-Romain opened this issue Apr 10, 2018 · 4 comments
Closed

Additionnal issues with sanitizer #18

Jean-Romain opened this issue Apr 10, 2018 · 4 comments

Comments

@Jean-Romain
Copy link
Collaborator

Jean-Romain commented Apr 10, 2018

@floriandeboissieu

I released rlas. Every regular tests are good but I received the following email from Prof. Bryan Ripley

See https://cran.r-project.org/web/checks/check_results_rlas.html (which is still updating).

This version has introduced serious 'Additional issues'. Please correct ASAP once all the results have been updated, and before Apr 23 to safely retain the package on CRAN.

Looking in the logs, it is an error with the memory sanitizer. ASAN and USBAN are diffcult tools to use. I'm definitively not able to reproduce and rhub::check_with_sanitizers is ok. Thus it is impossible to reproduce.

The good new is that the error on CRAN is informative:

writeLAS.cpp:383:48: runtime error: nan is outside the range of representable values of type 'int'
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior writeLAS.cpp:383:48 in

So in my opinion it is related to no_data. I'm not fully comfortable with this part. You are the main author of this code. Could you please check that.

  • Do we need to manually replace the NA by the no_data value (l.363)? And vis versa or is it managed internally by LASlib. I guess we have to do it manually.
  • Do we really need to rescale no_data (l.144)?

The support of NA is maybe not entirely correct.

@Jean-Romain
Copy link
Collaborator Author

And we have another one with GCC

 ==48816==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60300005add2 at pc 0x7f6c33cba36e bp 0x7ffed8038b90 sp 0x7ffed8038338
  READ of size 19 at 0x60300005add2 thread T0
      #0 0x7f6c33cba36d  (/lib64/libasan.so.4+0x5136d)
      #1 0x7f6c301ea457 in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::basic_string(char const*, std::allocator<char> const&) (/lib64/libstdc++.so.6+0x128457)
      #2 0x7f6c21c14356 in vlrsreader(LASheader*) /data/gannet/ripley/R/packages/tests-gcc-SAN/rlas/src/readheader.cpp:215

@floriandeboissieu
Copy link
Contributor

floriandeboissieu commented Apr 10, 2018 via email

@Jean-Romain
Copy link
Collaborator Author

This commit make the code much more readable and ensure that NA will never be written in the file. I hope it fixes clang-ASAN and clang-USBAN errors.

gcc-ASAN still don't like std::string(reinterpret_cast< char const* >(U8*))

Jean-Romain added a commit that referenced this issue Apr 11, 2018
This is expected to fix gcc ASAN error
@Jean-Romain
Copy link
Collaborator Author

This commit disable some features. I'm not able to convert U8* into string without using reinterpret_cast< char const* >. If these lines are really responsible of the ASAN error I'm not able to fix I remove the support for now.

Jean-Romain added a commit that referenced this issue Dec 31, 2018
Jean-Romain added a commit that referenced this issue Dec 31, 2018
This is expected to fix gcc ASAN error


Former-commit-id: d9968d6
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants