root icon indicating copy to clipboard operation
root copied to clipboard

Check whether ROOTSYS is defined in rootcling_stage1.

Open chilikink opened this issue 3 years ago • 4 comments

This Pull request:

Changes or fixes:

It is not checked in the existing code whether getenv("ROOTSYS") returns NULL. It does so if the environment variable is not defined; it is exactly what happens during building here. Then std::string gets initialized from the C string from address 0, which may result in a segmentation fault.

Checklist:

  • [ x ] tested changes locally
  • [ ] updated the docs (if necessary)

This PR fixes #

chilikink avatar Jun 16 '22 20:06 chilikink

Can one of the admins verify this patch?

phsft-bot avatar Jun 16 '22 20:06 phsft-bot

That's a symptom of another issue that we also need to understand. rootcling_stage1 is supposed to putenv ROOTSYS itself (in SetRootSys() in core/dictgen/src/rootcling_impl.cxx); it should not need to be set from the "outside". Do you know why SetRootSys() fails to do what it's supposed to?

Axel-Naumann avatar Jun 17 '22 06:06 Axel-Naumann

Yes, this happened because I was trying to compile on an officially unsupported system. The function GetExePath() is not implemented and returns an empty string. It the new commit, an error message is printed in SetRootSys() in case of unknown executable path, and the missing implementation of GetExePath() is added. The program rootcling_stage1 now determines its ROOTSYS (build directory) successfully as confirmed by printing rootsys in GetIncludeDir(), although it then still fails reporting circular dependencies in the headers, but it is a separate issue.

chilikink avatar Jun 17 '22 08:06 chilikink

Hi @chilikink, what's the status of this PR? Is this tweak still needed, and so you still want to get it merged? If yes, please let us know and resolve the conflicts by re-basing the commits on master.

guitargeek avatar Jan 09 '24 14:01 guitargeek

Superseded by #12996. Thanks for the initial effort, I'm sure it was helpful for the authors of the linked PR!

guitargeek avatar Sep 09 '24 07:09 guitargeek