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

Fix for most Xcode/clang warnings on OSX #5

Merged
merged 1 commit into from
Jul 11, 2015

Conversation

floooh
Copy link
Contributor

@floooh floooh commented Jul 11, 2015

This PR fixes the following warnings when compiled with latest clang in Xcode7 on OSX 10.11, there are 2 warnings remaining which I need advice on how to fix (see below).

The tests are running successfully.

In file included from /Users/floh/projects/glslang/SPIRV/SPVRemapper.cpp:37:
/Users/floh/projects/glslang/SPIRV/doc.h:217:9: warning: field 'resultPresent' will be initialized after field 'opDesc' [-Wreorder]
        resultPresent(true),       // most normal, only exceptions have to be spelled out

/Users/floh/projects/glslang/SPIRV/SpvBuilder.cpp:1217:5: warning: variable 'resultType' is used uninitialized whenever switch default is taken [-Wsometimes-uninitialized]
    default:
    ^~~~~~~
/Users/floh/projects/glslang/SPIRV/SpvBuilder.cpp:1221:57: note: uninitialized use occurs here
    Instruction* query = new Instruction(getUniqueId(), resultType, opCode);
                                                        ^~~~~~~~~~
/Users/floh/projects/glslang/SPIRV/SpvBuilder.cpp:1178:18: note: initialize the variable 'resultType' to silence this warning
    Id resultType;
                 ^
                  = 0

In file included from /Users/floh/projects/glslang/SPIRV/disassemble.cpp:53:
/Users/floh/projects/glslang/SPIRV/doc.h:217:9: warning: field 'resultPresent' will be initialized after field 'opDesc' [-Wreorder]
        resultPresent(true),       // most normal, only exceptions have to be spelled out

/Users/floh/projects/glslang/glslang/MachineIndependent/Initialize.cpp:1057:31: warning: '&&' within '||' [-Wlogical-op-parentheses]
    if (profile != EEsProfile && version >= 150 || esBarrier)
        ~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~ ~~
/Users/floh/projects/glslang/glslang/MachineIndependent/Initialize.cpp:1057:31: note: place parentheses around the '&&' expression to silence this warning
    if (profile != EEsProfile && version >= 150 || esBarrier)
                              ^
        (                                      )


In file included from /Users/floh/projects/glslang/glslang/MachineIndependent/ParseHelper.cpp:44:
/Users/floh/projects/glslang/glslang/MachineIndependent/preprocessor/PpContext.h:89:28: warning: field 'ival' will be initialized after field 'space' [-Wreorder]
    TPpToken() : token(0), ival(0), space(false), dval(0.0), atom(0)

/Users/floh/projects/glslang/glslang/MachineIndependent/Scan.cpp:688:48: warning: '&&' within '||' [-Wlogical-op-parentheses]
        if (parseContext.profile == EEsProfile && parseContext.version >= 310 ||
            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ~~
/Users/floh/projects/glslang/glslang/MachineIndependent/Scan.cpp:688:48: note: place parentheses around the '&&' expression to silence this warning
        if (parseContext.profile == EEsProfile && parseContext.version >= 310 ||
                                               ^
            (                                                                )

/Users/floh/projects/glslang/glslang/MachineIndependent/Versions.cpp:455:17: warning: enumeration values 'EBhMissing', 'EBhDisable', and 'EBhDisablePartial' not handled in switch [-Wswitch]
        switch (getExtensionBehavior(extensions[i])) {
                ^

There are 2 warnings remaining, these happen because a constant 0xFFFFFFFF is compared against an enum which doesn't contain such a value. To fix this I would need advice what your preferred fix would be, for instance, adding an 'Invalid' entry to the enum which has value 0xFFFFFFFF, or performing a cast of the enum to unsigned int inside the if()...?

/Users/floh/projects/glslang/SPIRV/GlslangToSpv.cpp:2446:13: warning: comparison of constant 4294967295 with expression of type 'spv::Decoration' is always true [-Wtautological-constant-out-of-range-compare]
    if (dec != spv::BadValue)
        ~~~ ^  ~~~~~~~~~~~~~
/Users/floh/projects/glslang/SPIRV/GlslangToSpv.cpp:2452:13: warning: comparison of constant 4294967295 with expression of type 'spv::Decoration' is always true [-Wtautological-constant-out-of-range-compare]
    if (dec != spv::BadValue)
        ~~~ ^  ~~~~~~~~~~~~~

- member initializing order in some constructors
- missing default branches in switch-case
- uninitialized variable if switch-case default (uncritical because
  program would exit)
- && and || brace warnings in if()
johnkslang added a commit that referenced this pull request Jul 11, 2015
Fix for most Xcode/clang warnings on OSX
@johnkslang johnkslang merged commit 549c293 into KhronosGroup:master Jul 11, 2015
@floooh floooh deleted the fix-clang-warnings branch July 12, 2015 11:17
@mre4ce
Copy link
Contributor

mre4ce commented Jun 26, 2016

This warning still exists when compiling on Linux as well:

GlslangToSpv.cpp:1973:33: warning: comparison of constant 4294967295 with expression of type 'spv::BuiltIn' is always true [-Wtautological-constant-out-of-range-compare]
                if (builtIn != spv::BadValue)
                    ~~~~~~~ ^  ~~~~~~~~~~~~~

This is not just a warning but a potential bug. The underlying type of an unscoped enum is an implementation-defined integral type capable of representing all values of the enum.

Considering that spv::Builtin only represent a limit number of enum values the underlying type could be 16-bit or even 8-bit in which case the comparison to 0xFFFFFFFF (spv::BadValue) will always be false.

johnkslang pushed a commit that referenced this pull request Sep 3, 2019
Sync code from KhronosGroup/glslang master
glebm added a commit to glebm/glslang that referenced this pull request Jan 30, 2021
UBSAN rightly complains on `push_front`:

    glslang/MachineIndependent/localintermediate.h:100:8: runtime error: load of value 160, which is not a valid value for type 'bool'
    #0 in glslang::TCall::TCall(glslang::TCall&&) glslang/MachineIndependent/localintermediate.h:100
    KhronosGroup#1 in void __gnu_cxx::new_allocator<std::_List_node<glslang::TCall> >::construct<glslang::TCall, glslang::TCall>(glslang::TCall*, glslang::TCall&&) /usr/include/c++/10/ext/new_allocator.h:150
    KhronosGroup#2 in void std::allocator_traits<std::allocator<std::_List_node<glslang::TCall> > >::construct<glslang::TCall, glslang::TCall>(std::allocator<std::_List_node<glslang::TCall> >&, glslang::TCall*, glslang::TCall&&) /usr/include/c++/10/bits/alloc_traits.h:512
    KhronosGroup#3 in std::_List_node<glslang::TCall>* std::__cxx11::list<glslang::TCall, std::allocator<glslang::TCall> >::_M_create_node<glslang::TCall>(glslang::TCall&&) (...)
    KhronosGroup#4 in void std::__cxx11::list<glslang::TCall, std::allocator<glslang::TCall> >::_M_insert<glslang::TCall>(std::_List_iterator<glslang::TCall>, glslang::TCall&&) /usr/include/c++/10/bits/stl_list.h:1911
    KhronosGroup#5 in std::__cxx11::list<glslang::TCall, std::allocator<glslang::TCall> >::push_front(glslang::TCall&&) /usr/include/c++/10/bits/stl_list.h:1167
    KhronosGroup#6 in glslang::TIntermediate::addToCallGraph(TInfoSink&, std::__cxx11::basic_string<char, std::char_traits<char>, glslang::pool_allocator<char> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, glslang::pool_allocator<char> > const&) glslang/MachineIndependent/Intermediate.cpp:2860

What happens here:

1. TCall's bool fields are not initialized on construction.
2. `push_front` move the `TCall` passed into it.
3. The move constructor copies unitialized bool, which may have an
   out-of-range value.

What this fix does:

Calls `emplace_back` to ensure no copy/move constructor is called.

Fixes KhronosGroup#2222
Refs KhronosGroup#2112
glebm added a commit to glebm/glslang that referenced this pull request Jan 30, 2021
UBSAN rightly complains on `push_front` here:

    glslang/MachineIndependent/localintermediate.h:100:8: runtime error: load of value 160, which is not a valid value for type 'bool'
    #0 in glslang::TCall::TCall(glslang::TCall&&) glslang/MachineIndependent/localintermediate.h:100
    KhronosGroup#1 in void __gnu_cxx::new_allocator<std::_List_node<glslang::TCall> >::construct<glslang::TCall, glslang::TCall>(glslang::TCall*, glslang::TCall&&) /usr/include/c++/10/ext/new_allocator.h:150
    KhronosGroup#2 in void std::allocator_traits<std::allocator<std::_List_node<glslang::TCall> > >::construct<glslang::TCall, glslang::TCall>(std::allocator<std::_List_node<glslang::TCall> >&, glslang::TCall*, glslang::TCall&&) /usr/include/c++/10/bits/alloc_traits.h:512
    KhronosGroup#3 in std::_List_node<glslang::TCall>* std::__cxx11::list<glslang::TCall, std::allocator<glslang::TCall> >::_M_create_node<glslang::TCall>(glslang::TCall&&) (...)
    KhronosGroup#4 in void std::__cxx11::list<glslang::TCall, std::allocator<glslang::TCall> >::_M_insert<glslang::TCall>(std::_List_iterator<glslang::TCall>, glslang::TCall&&) /usr/include/c++/10/bits/stl_list.h:1911
    KhronosGroup#5 in std::__cxx11::list<glslang::TCall, std::allocator<glslang::TCall> >::push_front(glslang::TCall&&) /usr/include/c++/10/bits/stl_list.h:1167
    KhronosGroup#6 in glslang::TIntermediate::addToCallGraph(TInfoSink&, std::__cxx11::basic_string<char, std::char_traits<char>, glslang::pool_allocator<char> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, glslang::pool_allocator<char> > const&) glslang/MachineIndependent/Intermediate.cpp:2860

What happens here:

1. TCall's bool fields are not initialized on construction.
2. `push_front` move the `TCall` passed into it.
3. The move constructor copies unitialized bool, which may have an
   out-of-range value.

What this fix does:

Calls `emplace_back` to ensure no copy/move constructor is called.

Fixes KhronosGroup#2222
Refs KhronosGroup#2112
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.

3 participants