#274 Compile with clang on OSX/XCode 7.2

[Daniel Kulp]

When building the latest in SVN on OSX using clang, there are a couple of minor compile errors. patch provided to fix them. (this allows the "core" stuff to compile)

[Teodor Petrov]

Can you explain why the changes are needed? Or even post the build log?
According to wikipedia OSX is LP64 so it should be defined.
Are you building a 32bit app?
What version of wx are you using?
And why an empty destructor is needed?

C::B compiles fine on linux when using clang. What clang version are you using?

[Daniel Kulp]

Attached a section of the build log.

64bit build on OSX. (no-one should be doing 32bit builts on OSX anymore) Built agains the latest wxWidgets from the 3.0.x branch (needed for fixes to various dialogs to allow it to run on Yosemite).

dkulp@MacBookPro ~/working/codeblocks $ clang++ --version
Apple LLVM version 7.0.2 (clang-700.1.81)
Target: x86_64-apple-darwin15.3.0
Thread model: posix

The empty destructor is needed for the "throw()". A couple of the fields have throws in their destructors so the destructor needs the throw() as well, I think.

[Daniel Kulp]

FYI: this is the clang that is installed via the latest releases from XCode from the Mac AppStore.

[Teodor Petrov]

Interesting. My versions of ~wxThread has nothing that mentions throwing in the declaration. I've checked 2.8, 3.0 and 3.1 on linux. And as far as I can see the header is common for all OSes. Can you check the declaration of the destructor in your version?

[Daniel Kulp]

The destructor for the CountedPtr has a throw() on it. WorkerThread has a field of "CountedPtr<wxsemaphore> m_semaphore" and thus that destructor would be called during the destruction of the WorkerThread. That's where the throw would potentially be coming from.</wxsemaphore>

[Teodor Petrov]

And I really doubt that you're building in 64 bit mode. See this discussion for details http://stackoverflow.com/questions/2383520/on-mac-os-x-in-c-on-a-64-bit-cpu-is-there-a-type-that-is-64-bits

If you search for sizeof(long) and sizeof(long long) you'll see that they are both 8 bytes. So they should be the same type... or it is the difference caused by the uint64_t definition?

[Daniel Kulp]

They are the same size, but different types. Code like:

    unsigned long i = 0;
    unsigned long *foo = &i;
    unsigned long long *bar = foo;

results in the same error. "long" and "long long" are both sizeof(..) == 8, but are different types and thus cannot be converted without a cast. On osx, uint64_t is defined as:

typedef unsigned long long uint64_t;

and thus is not an "unsigned long" so the direct pointer assignment doesn't work.

That said, a simple cast like:

if (address.ToULong((unsigned long*)&result, 16))

may also work just as well and would likely work for all the LP64 platforms.

[Daniel Kulp]

Actually, CountedPtr's destructor doesn't need the throws() on it as it doesn't throw anything. Removing that from the destructor allows things to compile as well and then WorkerThread doesn't need the empty destructor.

[Daniel Kulp]

Simplified patch that just removes the throws() and uses a cast for LP64. Compiles fine on OSX.

[Teodor Petrov]

Thanks, but I've redone the whole function using stringsteam. I'll post a patch later.

[Teodor Petrov]

This is the patch:

Index: src/sdk/debuggermanager.cpp
===================================================================
--- src/sdk/debuggermanager.cpp     (revision 10669)
+++ src/sdk/debuggermanager.cpp     (working copy)
@@ -472,29 +472,10 @@ uint64_t cbDebuggerStringToAddress(const wxString &address)
 {
     if (address.empty())
         return 0;
-#if defined(__WXMSW__)
-    // Workaround for the 'ToULongLong' bug in wxWidgets 2.8.12
-#if wxCHECK_VERSION(2, 8, 12)
-    return strtoull(address.mb_str(), nullptr, 16);
-#else
+    std::istringstream s(address.utf8_str().data());
     uint64_t result;
-    if (address.ToULongLong(&result, 16))
-        return result;
-    else
-        return 0;
-#endif // wxCHECK_VERSION
-#else
-    uint64_t result;
-    // different sizes of long/long long on 64bit/32bit need different conversions
-#ifdef __LP64__
-    if (address.ToULong(&result, 16))
-#else
-    if (address.ToULongLong(&result, 16))
-#endif // __LP64__
-        return result;
-    else
-        return 0;
-#endif
+    s >> result;
+    return (s.fail() ? 0 : result);
 }

 wxString cbDebuggerAddressToString(uint64_t address)

Can you try it and report if it works?

[Mojca Miklavec]

Before I noticed this thread I came up with the following patch:

--- a/src/sdk/debuggermanager.cpp
+++ b/src/sdk/debuggermanager.cpp
@@ -484,14 +484,9 @@ uint64_t cbDebuggerStringToAddress(const wxString &address)
         return 0;
 #endif // wxCHECK_VERSION
 #else
-    uint64_t result;
-    // different sizes of long/long long on 64bit/32bit need different conversions
-#ifdef __LP64__
-    if (address.ToULong(&result, 16))
-#else
+    wxULongLong_t result;
     if (address.ToULongLong(&result, 16))
-#endif // __LP64__
-        return result;
+        return (uint64_t)result;
     else
         return 0;
 #endif

But I can test the solution with istringstream. Is there any particular test that I could run (other than just checking whether it compiles)?

[Mojca Miklavec]

One problem I'm worried about is the base of the number. I have no clue what the code does or where it is used, but here's a minimal example that shows a potential problem:

uint64_t cbDebuggerStringToAddressA(const wxString &address)
{
    if (address.empty())
        return 999;
    std::istringstream s(address.utf8_str().data());

    uint64_t result;
    s >> result;
    return (s.fail() ? 777 : result);
}

uint64_t cbDebuggerStringToAddressB(const wxString &address)
{
    if (address.empty())
        return 999;

    wxULongLong_t result=0;
    if (address.ToULongLong(&result, 16))
        return (uint64_t)result;

    return 888;
}

int main()
{
    printf("%llu %llu %llu\n", 
        cbDebuggerStringToAddressA(wxString("12A")),
        cbDebuggerStringToAddressA(wxString("15")),
        cbDebuggerStringToAddressA(wxString("AB"))
        );
    printf("%llu %llu %llu\n", 
        cbDebuggerStringToAddressB(wxString("12A")),
        cbDebuggerStringToAddressB(wxString("15")),
        cbDebuggerStringToAddressB(wxString("AB"))
        );
    return 0;
}

The result is:

12 15 777
298 21 171

[Mojca Miklavec]

But I can use

s >> std::hex >> result;

and then at least the results seem to be the same. I only played with the minimal example, I didn't try to compile the code and test it in a real worl scenario.

[Teodor Petrov]

Yeah, it seems to be an omission on my side. I'll test it more later today.

[Daniel Kulp]

With the std::hex seems to work OK.

[Teodor Petrov]

Fixed in trunk. If there are more issues please open new tickets.