Modify

Ticket #8817 (closed Bugs: fixed)

Opened 9 months ago

Last modified 8 months ago

Boost Thread Windows CE _createthreadex handling breaks mingw w64

Reported by: Andrew Ho <helloworld922@…> Owned by: viboes
Milestone: Boost 1.55.0 Component: thread
Version: Boost 1.54.0 Severity: Problem
Keywords: MinGW uintptr_t Cc: jim@…

Description

The applicable code:

#ifndef BOOST_HAS_THREADEX
// Windows CE doesn't define _beginthreadex
typedef void* uintptr_t;
// ... more code
#endif

For some reason mingw-w64 will evaluate this code. This redefines uintptr_t (as well as _beginthreadex related code), which causes the build to fail.

 This post has a solution which avoids defining uintptr_t at all, and is targeted specifically for Windows CE rather than some arbitrary BOOST_HAS_THREADEX macro define.

I've created a patch which incorporates the fix and it does compile with mingw-w64 (mingw-builds, gcc 4.8.1 targeting windows x64), and doesn't break building with VS2012 (really shouldn't break any VS builds targeting Win32).

I don't know when the problem first occurs, but I do know that it fails in 1.53.0 all the way to trunk (my current working directory is r85020).

Attachments

thread.cpp.patch Download (2.4 KB) - added by Andrew Ho <helloworld922@…> 9 months ago.
patch for libs\thread\src\win32\thread.cpp
wince.patch Download (3.8 KB) - added by Andrew Ho <helloworld922@…> 9 months ago.
patch which fixes boost threads and also moves wince configs to win32.hpp
minimal-thread.cpp.patch Download (699 bytes) - added by Jim Bell <jim@…> 8 months ago.
Minimal patch to libs/thread/src/win32/thread.cpp: include <boost/cstdint.hpp> and yank the uintptr_t typedef.
mingw.config.patch Download (1.6 KB) - added by Andrew Ho <helloworld922@…> 8 months ago.
patch only for config detection with MinGW and WinCE

Change History

Changed 9 months ago by Andrew Ho <helloworld922@…>

patch for libs\thread\src\win32\thread.cpp

Changed 9 months ago by Andrew Ho <helloworld922@…>

patch which fixes boost threads and also moves wince configs to win32.hpp

comment:1 Changed 9 months ago by Andrew Ho <helloworld922@…>

After digging through the Boost configs, it seems that these defines are part of visualc.hpp:

#if defined(_WIN32_WCE) || defined(UNDER_CE)
// Windows CE does not have a conforming signature for swprintf
#  define BOOST_NO_SWPRINTF
#endif

// we have ThreadEx or GetSystemTimeAsFileTime unless we're running WindowsCE
#if !defined(_WIN32_WCE) && !defined(UNDER_CE)
#  define BOOST_HAS_THREADEX
#  define BOOST_HAS_GETSYSTEMTIMEASFILETIME
#endif

This probably should be included in win32.hpp, or perhaps in a new platform config file wince.hpp.

Perhaps this is a better option? I still think the changed no _beginthreadex may work betterh, though, as we probably shouldn't be typdef'ing uintptr_t anyways.

Added an updated patch which fixes both win32/thread.hpp as well as the threadex wince platform config.

comment:2 Changed 9 months ago by viboes

  • Owner changed from anthonyw to johnmaddock
  • Component changed from thread to config

I think this should be fixed in Boost.Config.

comment:3 Changed 9 months ago by Andrew Ho <helloworld922@…>

The problem is in Boost.Config and Boost.Thread.

Obviously the work-arounds for Windows CE should not be used when compiling with MinGW (unless targeting Windows CE specifically), so Boost.Config must be changed.

IMO Boost.Thread should also be changed because defining uintptr_t really shouldn't be done at all. I would imagine any modern compiler targeting Windows CE might have uintptr_t defined (it is standard C++ after all), in which case the compile will fail again.

comment:4 Changed 8 months ago by Jim Bell <jim@…>

  • Cc jim@… added

Changed 8 months ago by Jim Bell <jim@…>

Minimal patch to libs/thread/src/win32/thread.cpp: include <boost/cstdint.hpp> and yank the uintptr_t typedef.

comment:5 follow-up: ↓ 9 Changed 8 months ago by Jim Bell <jim@…>

  • Keywords MinGW uintptr_t added
  • Owner changed from johnmaddock to anthonyw
  • Component changed from config to thread

Added a minimal patch to libs/thread/src/win32/thread.cpp which I verified to work with MinGW gcc 4.8, and both boost trunk and release SVN branches. Using <boost/cstdint.hpp> for the typedef for uintptr_t maximizes confidence that it will work for WinCE as well. (And should WinCE fail, pushes the fix from thread to config where it belongs.) With this change being only to thread.cpp, I'm changing the component from config back to thread.

Changed 8 months ago by Andrew Ho <helloworld922@…>

patch only for config detection with MinGW and WinCE

comment:6 Changed 8 months ago by Andrew Ho <helloworld922@…>

Should the code for detecting BOOST_NO_SWPRINTF, BOOST_HAS_THREADEX, and BOOST_HAS_GETSYSTEMTIMEASFILETIME be moved to win32.hpp (from visualc.hpp)? These are platform-specific defines, not compiler-specific.

You should be able to verify this with Mingw testing this code:

#include <boost/config>
#include <iostream>

int main(void)
{
#ifndef BOOST_HAS_THREADEX
    // This should be true for any compiler targeting Windows and can use the WinAPI, which MinGW can
    std::cout << "didn't detect BOOST_HAS_THREADEX" << std::endl;
#else
    std::cout << "has BOOST_HAS_THREADEX" << std::endl;
#endif
}

As-is, MinGW will still not detect these correctly because it doesn't (and shouldn't) include visualc.hpp. However, when targeting Windows it should and will include win32.hpp and detection will work as intended.

I've added a smaller patch which only applies these fixes to Config, so applying minimal-thread.cpp.patch and mingw.config.patch should fully fix Boost.Config and Boost.Thread.

comment:7 Changed 8 months ago by Jim Bell <jim@…>

Andrew: I have to say I didn't look too closely at your proposed solution, but was working backwards from (a lot of) failed regression tests and found this ticket. I'd say (1) you might want to split this into a separate ticket for config, and (2) present it as a specific failed regression test, or a missing regression test.

comment:8 Changed 8 months ago by Jim Bell <jim@…>

I'm pretty sure was caused by [82777] on thread.cpp, which changed the <thread.hpp> include to <thread_only.hpp>. Looking for a way to change this ticket's owner, but apparently don't have permission. Will look for him on lists (viboes) and contact directly.

comment:9 in reply to: ↑ 5 Changed 8 months ago by viboes

Replying to Jim Bell <jim@…>:

Added a minimal patch to libs/thread/src/win32/thread.cpp which I verified to work with MinGW gcc 4.8, and both boost trunk and release SVN branches. Using <boost/cstdint.hpp> for the typedef for uintptr_t maximizes confidence that it will work for WinCE as well. (And should WinCE fail, pushes the fix from thread to config where it belongs.) With this change being only to thread.cpp, I'm changing the component from config back to thread.

This code was working well on mingw 32. What has been changed in mingw 64? Have you tested your patch on mingw 32?

comment:10 Changed 8 months ago by viboes

  • Owner changed from anthonyw to viboes
  • Status changed from new to assigned
  • Version changed from Boost Development Trunk to Boost 1.54.0
  • Milestone changed from To Be Determined to Boost 1.55.0

Committed revision [85540].

comment:11 Changed 8 months ago by Jim Bell <jim@…>

I'm reverting the trunk patch on my MinGW-gcc testers and will begin to re-cycle those tests.

comment:12 Changed 8 months ago by Jim Bell <jim@…>

It appears that all trunk tests have run with revisions above [85540], and without issue. I think this can go to the release branch and close the ticket.

comment:13 Changed 8 months ago by Andrew Ho <helloworld922@…>

I submitted the other portion of this issue with Boost.Config as #9095. This should allow proper detection of Windows platform features on compilers other than VS by Boost.Config.

comment:14 Changed 8 months ago by viboes

  • Status changed from assigned to closed
  • Resolution set to fixed

Committed revision [85621].

View

Add a comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
The resolution will be deleted. Next status will be 'reopened'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.