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.

Sign up for GitHub

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

Jump to bottom

Cygwin build fixes #1332

Merged
merged 3 commits into from Feb 11, 2020
Merged

Cygwin build fixes #1332

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2439b7e
Workaround for Cygwin build
okuoku Feb 9, 2020
5ea60bc
wasm-decompile: Silence -Wsign-compare
okuoku Feb 9, 2020
f3524be
wasm-objdump: Fix 4294967296 output on disasm
okuoku Feb 9, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 9 additions & 1 deletion CMakeLists.txt
View file
Open in desktop
Expand Up @@ -118,7 +118,15 @@ else ()
-Wuninitialized
)

set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++11 -Wold-style-cast")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wold-style-cast")
sbc100 marked this conversation as resolved.
Show resolved Hide resolved

if(NOT CYGWIN)
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++11")
else()
# On Cygwin, POSIX extensions are not visible with std=c++11
# because it defines __STRICT_ANSI__.
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=gnu++11")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be better to define _POSIX_SOURCE or something like that to signal that we want POSIX rather than asking for GNU extensions? I also don't think its a great idea to use different -std on different platforms.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, there's room for improvement. Currently, the only POSIX consumers are

wabt/src/color.cc

Line 69 in 4ba97c1

return isatty(fileno(file));
AND gtest. For the color.cc we'd just have _POSIX_C_SOURCE on it but I'm not sure how do we want to deal with gtest...

So I choose to limit gnu++11 to Cygwin and leave it as-is on the other (major) implementations such as glibc(Linux) or BSD libc.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would make sense to define _POSIX_C_SOURCE for the whole project (assuming that works) than turn on GNU extensions.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I'll try. I found gtest upstream also have a proposed patch https://github.com/google/googletest/pull/2653/files so we should be able to do the same here.

The only culprit seems strcasecmp which is also POSIX. I'll chase why Cygwin does not export it under _POSIX_C_SOURCE=200809L.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Filed: #1333

endif()

if (WERROR)
add_definitions(-Werror)
Expand Down
2 changes: 1 addition & 1 deletion src/binary-reader-objdump.cc
View file
Open in desktop
Expand Up @@ -598,7 +598,7 @@ Result BinaryReaderObjdumpDisassemble::OnOpcodeUint32(uint32_t value) {
Result BinaryReaderObjdumpDisassemble::OnOpcodeUint32Uint32(uint32_t value,
uint32_t value2) {
Offset immediate_len = state->offset - current_opcode_offset;
LogOpcode(immediate_len, "%lu %lu", value, value2);
LogOpcode(immediate_len, "%u %u", value, value2);
return Result::Ok;
}

Expand Down
2 changes: 1 addition & 1 deletion src/decompiler.cc
View file
Open in desktop
Expand Up @@ -302,7 +302,7 @@ struct Decompiler {
if (se.opcode == Opcode::I32Shl &&
const_exp.etype == ExprType::Const) {
auto& ce = *cast<ConstExpr>(const_exp.e);
if (ce.const_.type == Type::I32 && (1 << ce.const_.u32) == align) {
if (ce.const_.type == Type::I32 && (1U << ce.const_.u32) == align) {
// Pfew, case detected :( Lets re-write this in Haskell.
// TODO: we're decompiling these twice.
// The thing to the left of << is going to be part of the index.
Expand Down