ci: Add Github Action for codeql #524

axic · 2020-09-08T23:07:10Z

No description provided.

codecov · 2020-10-07T11:52:49Z

Codecov Report

Merging #524 (7b700be) into master (1fa057b) will decrease coverage by 0.13%.
The diff coverage is n/a.

❗ Current head 7b700be differs from pull request most recent head 7f19e0e. Consider uploading reports for the commit 7f19e0e to get more accurate results

@@            Coverage Diff             @@
##           master     #524      +/-   ##
==========================================
- Coverage   99.26%   99.12%   -0.14%     
==========================================
  Files          88       85       -3     
  Lines       13268    13010     -258     
==========================================
- Hits        13170    12896     -274     
- Misses         98      114      +16

Flag	Coverage Δ
rust	`99.90% <0.00%> (+1.42%)`	⬆️
spectests	`89.98% <0.00%> (?)`
unittests	`98.94% <0.00%> (?)`
unittests-32	`99.03% <0.00%> (-0.29%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
tools/wasi/wasi.cpp	`60.18% <0.00%> (-32.41%)`	⬇️
lib/fizzy/capi.cpp	`95.41% <0.00%> (-0.28%)`	⬇️
lib/fizzy/execute.cpp	`99.29% <0.00%> (-0.02%)`	⬇️
lib/fizzy/execute.hpp	`100.00% <0.00%> (ø)`
lib/fizzy/instructions.cpp	`100.00% <0.00%> (ø)`
lib/fizzy/execution_context.hpp	`100.00% <0.00%> (ø)`
test/unittests/execute_test.cpp	`100.00% <0.00%> (ø)`
test/unittests/instantiate_test.cpp	`100.00% <0.00%> (ø)`
test/unittests/capi_execute_test.cpp	`100.00% <0.00%> (ø)`
test/unittests/floating_point_utils_test.cpp	`100.00% <0.00%> (ø)`
... and 8 more

axic · 2021-01-27T00:20:40Z

codeql is failing, it seems to be using gcc 7.5.0:

Scanning dependencies of target fizzy
[  1%] Building CXX object lib/fizzy/CMakeFiles/fizzy.dir/asserts.cpp.o
[  2%] Building CXX object lib/fizzy/CMakeFiles/fizzy.dir/capi.cpp.o
In file included from /usr/include/c++/7/vector:62:0,
                 from /home/runner/work/fizzy/fizzy/lib/fizzy/types.hpp:12,
                 from /home/runner/work/fizzy/fizzy/lib/fizzy/module.hpp:7,
                 from /home/runner/work/fizzy/fizzy/lib/fizzy/instantiate.hpp:10,
                 from /home/runner/work/fizzy/fizzy/lib/fizzy/execute.hpp:9,
                 from /home/runner/work/fizzy/fizzy/lib/fizzy/capi.cpp:6:
/usr/include/c++/7/bits/stl_construct.h: In instantiation of ‘void std::_Construct(_T1*, _Args&& ...) [with _T1 = fizzy::Import; _Args = {const fizzy::Import&}]’:
/usr/include/c++/7/bits/stl_uninitialized.h:83:18:   required from ‘static _ForwardIterator std::__uninitialized_copy<_TrivialValueTypes>::__uninit_copy(_InputIterator, _InputIterator, _ForwardIterator) [with _InputIterator = __gnu_cxx::__normal_iterator<const fizzy::Import*, std::vector<fizzy::Import> >; _ForwardIterator = fizzy::Import*; bool _TrivialValueTypes = false]’
/usr/include/c++/7/bits/stl_uninitialized.h:134:15:   required from ‘_ForwardIterator std::uninitialized_copy(_InputIterator, _InputIterator, _ForwardIterator) [with _InputIterator = __gnu_cxx::__normal_iterator<const fizzy::Import*, std::vector<fizzy::Import> >; _ForwardIterator = fizzy::Import*]’
/usr/include/c++/7/bits/stl_uninitialized.h:289:37:   required from ‘_ForwardIterator std::__uninitialized_copy_a(_InputIterator, _InputIterator, _ForwardIterator, std::allocator<_Tp>&) [with _InputIterator = __gnu_cxx::__normal_iterator<const fizzy::Import*, std::vector<fizzy::Import> >; _ForwardIterator = fizzy::Import*; _Tp = fizzy::Import]’
/usr/include/c++/7/bits/stl_vector.h:331:31:   required from ‘std::vector<_Tp, _Alloc>::vector(const std::vector<_Tp, _Alloc>&) [with _Tp = fizzy::Import; _Alloc = std::allocator<fizzy::Import>]’
/home/runner/work/fizzy/fizzy/lib/fizzy/module.hpp:14:8:   required from ‘typename std::_MakeUniq<_Tp>::__single_object std::make_unique(_Args&& ...) [with _Tp = fizzy::Module; _Args = {const fizzy::Module&}; typename std::_MakeUniq<_Tp>::__single_object = std::unique_ptr<fizzy::Module, std::default_delete<fizzy::Module> >]’
/home/runner/work/fizzy/fizzy/lib/fizzy/capi.cpp:359:69:   required from here
/usr/include/c++/7/bits/stl_construct.h:75:7: error: use of deleted function ‘fizzy::Import::Import(const fizzy::Import&)’
     { ::new(static_cast<void*>(__p)) _T1(std::forward<_Args>(__args)...); }
       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from /home/runner/work/fizzy/fizzy/lib/fizzy/module.hpp:7:0,
                 from /home/runner/work/fizzy/fizzy/lib/fizzy/instantiate.hpp:10,
                 from /home/runner/work/fizzy/fizzy/lib/fizzy/execute.hpp:9,
                 from /home/runner/work/fizzy/fizzy/lib/fizzy/capi.cpp:6:
/home/runner/work/fizzy/fizzy/lib/fizzy/types.hpp:325:8: note: ‘fizzy::Import::Import(const fizzy::Import&)’ is implicitly deleted because the default definition would be ill-formed:
 struct Import
        ^~~~~~
/home/runner/work/fizzy/fizzy/lib/fizzy/types.hpp:325:8: error: use of deleted function ‘fizzy::Import::<unnamed union>::<constructor>(const fizzy::Import::<unnamed union>&)’
/home/runner/work/fizzy/fizzy/lib/fizzy/types.hpp:331:5: note: ‘fizzy::Import::<unnamed union>::<constructor>(const fizzy::Import::<unnamed union>&)’ is implicitly deleted because the default definition would be ill-formed:
     {
     ^
/home/runner/work/fizzy/fizzy/lib/fizzy/types.hpp:333:16: error: union member ‘fizzy::Import::<unnamed union>::memory’ with non-trivial ‘constexpr fizzy::Memory::Memory(const fizzy::Memory&)’
         Memory memory;
                ^~~~~~
/home/runner/work/fizzy/fizzy/lib/fizzy/types.hpp:335:15: error: union member ‘fizzy::Import::<unnamed union>::table’ with non-trivial ‘constexpr fizzy::Table::Table(const fizzy::Table&)’
         Table table;
               ^~~~~
cc1plus: error: unrecognized command line option ‘-Wno-unknown-attributes’ [-Werror]
cc1plus: all warnings being treated as errors
make[2]: *** [lib/fizzy/CMakeFiles/fizzy.dir/capi.cpp.o] Error 1
make[1]: *** [lib/fizzy/CMakeFiles/fizzy.dir/all] Error 2
lib/fizzy/CMakeFiles/fizzy.dir/build.make:94: recipe for target 'lib/fizzy/CMakeFiles/fizzy.dir/capi.cpp.o' failed
CMakeFiles/Makefile2:441: recipe for target 'lib/fizzy/CMakeFiles/fizzy.dir/all' failed
make: *** [all] Error 2
Makefile:159: recipe for target 'all' failed
Error: Process completed with exit code 2.

@gumb0 @chfast do you think we can easily fix this? If it is messy, we should just mark old gcc unsupported (should also have the minimum compiler versions set in cmake).

gumb0 · 2021-01-28T10:37:47Z

@gumb0 @chfast do you think we can easily fix this? If it is messy, we should just mark old gcc unsupported (should also have the minimum compiler versions set in cmake).

Looks to be a limitation of std::optional in old GCC:

union is not allowed to have non-trivially constructed members
we have Import containing union containing Memory and Table that have Limits containing std::optional<uint32_t> max;
std::optional was (retroactively) prescribed in 2018 to have trivial constructors in case value it contains is trivially-constructed

http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/p0602r4.html
GCC implemented this fix in version 8.3 https://gcc.gnu.org/onlinedocs/gcc-10.2.0/libstdc++/manual/manual/status.html

I don't see any easy fix for this other than replacing std::optional with something else like bool+uint32_t, maybe it's not worth it.

axic · 2021-01-28T10:52:12Z

I'd vote for requiring gcc 8.3 as the minimum.

gumb0 · 2021-01-28T10:59:17Z

union is not allowed to have non-trivially constructed members

Actually this was true only till C++11, now we can define our own union constructors... I'll try something.

axic · 2021-03-29T22:05:38Z

lib/fizzy/execute.cpp

@@ -446,7 +446,7 @@ T fnearest(T value) noexcept
 }

 template <typename T>
-__attribute__((no_sanitize("float-divide-by-zero"))) inline constexpr T fdiv(T a, T b) noexcept
+[[gnu::no_sanitize("float-divide-by-zero")]] inline constexpr T fdiv(T a, T b) noexcept


@chfast is it useful merging these changes?

Apparently it doesn't work as the sanitizer task reports division-by-zero.

axic · 2021-03-29T22:06:18Z

@gumb0 @chfast do we want to specify minimum required compiler versions in cmake/readme? It seems gcc >=8.3 is required unless we want to merge this commit from @gumb0.

chfast · 2021-03-30T07:13:49Z

@gumb0 @chfast do we want to specify minimum required compiler versions in cmake/readme? It seems gcc >=8.3 is required unless we want to merge this commit from @gumb0.

No.

axic · 2021-03-31T21:38:15Z

Why not? This PR showed that fizzy was not compiling with gcc<8.3.

chfast · 2021-03-31T21:59:44Z

Why not? This PR showed that fizzy was not compiling with gcc<8.3.

If you want to do this, you need to create CI jobs with the discovered compiler version. I don't think this is worth the time right now.

This fixes Import being not copy-constructible in GCC older than 8.3, where std::optional is not trivially constructible.

axic force-pushed the codeql-action branch from e2d95b4 to 532d53f Compare October 7, 2020 11:14

axic force-pushed the codeql-action branch 2 times, most recently from 93633c5 to 7900c3f Compare October 7, 2020 12:16

axic force-pushed the codeql-action branch from 4fd2e8a to a4d8fe2 Compare January 27, 2021 00:14

gumb0 force-pushed the codeql-action branch from e17721c to 713e447 Compare January 28, 2021 11:13

axic force-pushed the codeql-action branch from b49e140 to 338230f Compare March 11, 2021 15:21

axic commented Mar 29, 2021

View reviewed changes

axic force-pushed the codeql-action branch from 338230f to 9040d68 Compare May 21, 2022 10:13

axic and others added 4 commits June 21, 2022 13:25

ci: Add Github Action for codeql

57492db

Run build

4e06a3f

Add explicit constructors to Import

53f827e

This fixes Import being not copy-constructible in GCC older than 8.3, where std::optional is not trivially constructible.

Convert no_sanitize attributes to C++ syntax

7f19e0e

axic force-pushed the codeql-action branch from 9040d68 to 7f19e0e Compare June 21, 2022 11:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ci: Add Github Action for codeql #524

ci: Add Github Action for codeql #524

axic commented Sep 8, 2020

codecov bot commented Oct 7, 2020 •

edited

Loading

axic commented Jan 27, 2021 •

edited

Loading

gumb0 commented Jan 28, 2021 •

edited

Loading

axic commented Jan 28, 2021

gumb0 commented Jan 28, 2021

axic Mar 29, 2021

chfast Mar 30, 2021

axic May 21, 2022

axic commented Mar 29, 2021

chfast commented Mar 30, 2021

axic commented Mar 31, 2021

chfast commented Mar 31, 2021

ci: Add Github Action for codeql #524

Are you sure you want to change the base?

ci: Add Github Action for codeql #524

Conversation

axic commented Sep 8, 2020

codecov bot commented Oct 7, 2020 • edited Loading

Codecov Report

axic commented Jan 27, 2021 • edited Loading

gumb0 commented Jan 28, 2021 • edited Loading

axic commented Jan 28, 2021

gumb0 commented Jan 28, 2021

axic Mar 29, 2021

Choose a reason for hiding this comment

chfast Mar 30, 2021

Choose a reason for hiding this comment

axic May 21, 2022

Choose a reason for hiding this comment

axic commented Mar 29, 2021

chfast commented Mar 30, 2021

axic commented Mar 31, 2021

chfast commented Mar 31, 2021

codecov bot commented Oct 7, 2020 •

edited

Loading

axic commented Jan 27, 2021 •

edited

Loading

gumb0 commented Jan 28, 2021 •

edited

Loading