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

[WIP] Travis: Enable UBSan with clang #337

Closed
wants to merge 3 commits into from

Conversation

@Algunenano
Copy link
Member

commented Nov 20, 2018

No description provided.

@Algunenano Algunenano force-pushed the Algunenano:travis_clang_usan branch 3 times, most recently from e291864 to bf11f80 Nov 20, 2018

@Algunenano

This comment has been minimized.

Copy link
Member Author

commented Nov 20, 2018

@dbaston @Komzpa What's the proper way to include a recent release of clang into the docker images?

It's not urgent since I'm expecting the tests to crash 😄

@Komzpa

This comment has been minimized.

Copy link
Member

commented Nov 20, 2018

@Algunenano commit it as dependency into Dockerfile here:

https://github.com/postgis/postgis-build-env

and get someone with push access to Dockerhub run python3 build.py. I can do this in the evening.

(If you want a fun engineering task, Travis has push rights and credentials set up, but build takes more than Travis limit - if you can chop it into quasi-independent stages faster tha then you can do it without me or @dbaston).

@Algunenano

This comment has been minimized.

Copy link
Member Author

commented Nov 20, 2018

I don't have write access, so here is the PR: postgis/postgis-build-env#2

(If you want a fun engineering task, Travis has push rights and credentials set up, but build takes more than Travis limit - if you can chop it into quasi-independent stages faster tha then you can do it without me or @dbaston).

"fun engineering task" 😸 😸 😸

@Komzpa

This comment has been minimized.

Copy link
Member

commented Nov 20, 2018

clang now in image. I restarted the build but it fails weirdly.

gcc: error: unrecognized command line option '-fsanitize-trap=undefined'; did you mean '-fsanitize=undefined'?
@Algunenano

This comment has been minimized.

Copy link
Member Author

commented Nov 21, 2018

I've changed it to use the old deprecated flag (that is common with gcc) and now it's working and crashing as expected. Thanks a lot @Komzpa

@Komzpa

This comment has been minimized.

Copy link
Member

commented Nov 21, 2018

Question is, why is it switching to gcc when clang was asked explicitly.

@Algunenano

This comment has been minimized.

Copy link
Member Author

commented Nov 21, 2018

Question is, why is it switching to gcc when clang was asked explicitly.

Because Postgres was compiled with GCC and PGXS enforces it. The parts of Postgis that deal with Postgresql itself are always built using Postgres flags (both CC and CFLAGS).

@Komzpa

This comment has been minimized.

Copy link
Member

commented Nov 21, 2018

In this case, would it be beneficial to build a second version of the image, where Postgres is built with clang, so we can also check for UB in that code with clang's ubsan?

@Algunenano

This comment has been minimized.

Copy link
Member Author

commented Nov 21, 2018

In this case, would it be beneficial to build a second version of the image, where Postgres is built with clang, so we can also check for UB in that code with clang's ubsan?

Yes, I haven't found any issue around that code in my local tests, but I wouldn't discard it in the future. I'd also take advantage of the change and enable JIT by building Postgres with --with-llvm.

@Algunenano Algunenano force-pushed the Algunenano:travis_clang_usan branch from 887452b to 768aeb3 Nov 22, 2018

@strk strk closed this in 4edcafa Nov 22, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.