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

Do not allocate zero size buffer. #200

Closed
wants to merge 1 commit into from

Conversation

cmcantalupo
Copy link
Contributor

Signed-off-by: Christopher M. Cantalupo christopher.m.cantalupo@intel.com

Signed-off-by: Christopher M. Cantalupo <christopher.m.cantalupo@intel.com>
@cmcantalupo
Copy link
Contributor Author

@bgoglin

I'm using hwloc in our project here:

https://github.com/geopm/geopm

We are getting a segfault in our tests on fedora 24:

https://build.opensuse.org/build/home:cmcantalupo/Fedora_24/x86_64/geopm/_log

Electric fence turned up the issue fixed by this patch and I think this is causing the segmentation fault in our tests. If the patch looks good, please accept the pull request.

The error can be reproduced with the following command if hwloc is installed with the prefix $HOME/build/hwloc and the command is run in the git root.

LD_PRELOAD=/usr/lib64/libefence.so LD_LIBRARY_PATH=$HOME/build/hwloc/lib:$LD_LIBRARY_PATH tests/hwloc/.libs/hwloc_topology_dup

Thanks,
Chris

@bgoglin
Copy link
Contributor

bgoglin commented Jul 21, 2016

Why does malloc(0) fail on Fedora 24? My glibc manual says that malloc(0) is valid.
Anyway, the patch is fine, but I don't see why it would cause a segfault. Your link to build.opensuse.org is unaccessible to anonymous users unfortunately.

@cmcantalupo
Copy link
Contributor Author

Agreed that malloc(0) is not strictly invalid, however, is it possible that the object created in this way is misused later in the code? I'm just validating that the patch fixes the problem I am seeing on the geopm fedora 24 tests in OBS. I'll get back here when I have the result.

@cmcantalupo
Copy link
Contributor Author

The patch does not change the geopm test failure on fedora 24, but it does stop electric fence from complaining. Thanks for the feedback.

@bgoglin
Copy link
Contributor

bgoglin commented Jul 21, 2016

Can you send more details about how to reproduce the failure or make the build log public ?

@cmcantalupo
Copy link
Contributor Author

Until I can prove otherwise I will assume the failure I'm looking into is due to a bug in geopm. The hwloc tests only fail when electric fence is monitoring for malloc(0) occurrences. The patch provided by this pull request does alleviate this issue, but as you mentioned, malloc(0) is not necessarily incorrect.

bgoglin pushed a commit that referenced this pull request Jul 21, 2016
Signed-off-by: Christopher M. Cantalupo <christopher.m.cantalupo@intel.com>

Not strictly required but avoids efence warnings. Refs #200.
bgoglin pushed a commit that referenced this pull request Jul 21, 2016
Signed-off-by: Christopher M. Cantalupo <christopher.m.cantalupo@intel.com>

Not strictly required but avoids efence warnings. Refs #200.

(cherry picked from commit 6cc37e8)
@bgoglin
Copy link
Contributor

bgoglin commented Jul 21, 2016

I applied your patch to master and v1.11 branches, so I am closing this PR. If you find a deeper issue causing the GEOPM segfault, feel free to reopen and/or open a new PR or issue.
As a thank you, I added GEOPM to the list of hwloc users on the website :)

@bgoglin bgoglin closed this Jul 21, 2016
@cmcantalupo
Copy link
Contributor Author

Thanks! Found the bug, in the end it was a bad interaction between googletest 1.7 and gcc6:

google/googletest#705

The fix is here:

cmcantalupo/geopm@bc1ba0b

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

Successfully merging this pull request may close these issues.

2 participants