X Tutup
The Wayback Machine - https://web.archive.org/web/20230317201151/https://github.com/github/codeql/issues/11790
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

CodeQL reports incorrect struct size for std::function #11790

Open
WilliamParks opened this issue Dec 30, 2022 · 6 comments
Open

CodeQL reports incorrect struct size for std::function #11790

WilliamParks opened this issue Dec 30, 2022 · 6 comments
Labels
C++ question Further information is requested

Comments

@WilliamParks
Copy link

I've got a test case where CodeQL reports different structure sizes than the actual compiler.

This is using a recent compile of LLVM 15.0.6, on an M1 Mac, as well as the default OS clang binary. I have a fresh install of CodeQL, version 2.11.6.
Notably, this only happens when using clang++, and not when I use g++.

Minimized source file:

#include <functional>
 
struct Wrapper {
    std::function<void(int)> foo;
};

int main()
{
    printf("size %ld\n", sizeof(Wrapper));
}

Compilation command:

codeql database create ./clang_db --overwrite --language=cpp --command="/Users/b/tools/llvm-project/build/bin/clang++ -std=c++17 ./test.cpp -o clang_test"

CodeQL query:

import cpp

from Class c
where c.getName() = "Wrapper"
select c.getName(), c.getSize()

CodeQL Output

Compiling query plan for /Users/b/projects/DataCov/codeql/temp.ql.
[1/1 comp 2.4s] Compiled /Users/b/projects/DataCov/codeql/temp.ql.
temp.ql: Evaluation completed (124ms).
|  col0   | col1 |
+---------+------+
| Wrapper |   48 |
Shutting down query evaluator.

Running clang_test

size 32

Notably, the g++ compiled version reports the same size as CodeQL.

@WilliamParks WilliamParks added the question Further information is requested label Dec 30, 2022
@WilliamParks
Copy link
Author

In playing around with this some more, I get different behavior for the compiled binary, in and out of codeql database create.


codeql database create ./gpp_db --overwrite --language=cpp --command="g++ -std=c++17 ./test.cpp -o gpp_test"
Initializing database at /Users/b/projects/foo/gpp_db.
Running build command: [g++, -std=c++17, ./test.cpp, -o, gpp_test]
Finalizing database at /Users/b/projects/foo/gpp_db.
Successfully created database at /Users/b/projects/foo/gpp_db.

./gpp_test
size 48

g++ -std=c++17 ./test.cpp -o gpp_test2
./gpp_test2
size 32

@WilliamParks
Copy link
Author

Going from a fully clean install on Ubuntu 22, both sizeof() and CodeQL produce the same results.

@jketema jketema self-assigned this Dec 31, 2022
@WilliamParks
Copy link
Author

I wrote a quick tool to compare DWARF and CodeQL results (https://github.com/WilliamParks/Struct-Check), and it seems std::function is the only improperly sized struct in the large project I've been working with

@jketema
Copy link
Contributor

jketema commented Jan 1, 2023

Hi @WilliamParks,

Thanks for your question. There are two things going on here:

  1. clang's target architecture detection fails when running the compilation under codeql database create, and it creates a x86-64 binary instead of a arm64 binary. This is due to a know issue: the component tracing the build is a x86-64 binary and not a universal binary. This explains the mismatch you observed while running the executables. To work around this please specify an explicit -target during compilation, e.g., -target arm64-apple-macos11. This will not affect the size reported by CodeQL though.
  2. The front-end we're using currently assumes x86-64 struct member alignment even when the target is arm64. One of the underlying structs of std::function does some alignment trickery causing the struct to be larger than expected for the arm64 target.

The second issue is in principle fixable. However, to properly prioritise this, would you mind explaining what your use-case is?

@WilliamParks
Copy link
Author

WilliamParks commented Jan 1, 2023

Hi @jketema,
Ahh, interesting! Thanks for passing that along. I didn't think to check what type of binary was being produced.

I'm using CodeQL static analysis as part of a fuzzing research project. I've been doing development on an arm64 mac, but it'll actually run on an x86-64 system. Because of that, I think this is low priority.

Thanks!

@jketema
Copy link
Contributor

jketema commented Jan 3, 2023

Thanks for sketching the context in which you use this. I've opened some internal issues to track the problems you observed.

@jketema jketema removed their assignment Jan 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C++ question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants
X Tutup