Opened 13 months ago

Last modified 7 months ago

#15138 reopened defect

Stacktrace doesn't work with non-ASCII or Unicode identifiers

Reported by: suzumizaki Owned by: vadz
Priority: normal Milestone:
Component: wxMSW Version: stable-latest
Keywords: Unicode call stack trace non-ASCII work-needed Cc:
Blocked By: Blocking:
Patch: yes

Description

Current stack-tracing code always fails if some functions have non-ASCII identifiers with wxMSW.

When the assertion-stop is made while calling the function with non-ASCII named, nonsense another stop will also be made. "Nonsense" one throws away the information we really need, which the first one has.

This invalid behavior depends on calling wxString::FromAscii(char*) with non-ASCII string in short. But with accuracy to say, we should use wide char (PCWSTR/PWSTR) versions of debugging API on Windows, not ANSI(MBCS) ones.

As you know, C++ specification allows non-ASCII identifiers. This means the names can be exists outside of wxWidgets and the outside one can be included into our stack traces.

I tried to fix and post the patch, but I don't know which environment we should check especially outdated operating systems/SDKs/develop environments. Anyone Help? Or what should I do anything else?

I checked by attached .cpp code (simply replace with "minimal.cpp") with:
1) Windows 8 Pro 64bit
2A) Visual C++ 2010 Express (with Windows SDK 7.1 for 64 bit)
2B) Visual Studio 2012 Express for Windows Desktop
note1) Both 2A and 2B, I tested only UNICODE build, but both 32bit and 64bit.
note2) All 1, 2A, 2B are working as Japanese Edition.

Attachments (7)

wx_trunk_msw_stacktrace_20130406.patch download (25.2 KB) - added by suzumizaki 13 months ago.
Patchs to trunk to resolve stack tracing problem with non-ASCII identifiers with wxMSW
stacktracetest.cpp download (7.3 KB) - added by suzumizaki 13 months ago.
C++ code for test. Please replace minimal.cpp, save as utf-8 or utf-16, build debug mode, and push button and check whether dialog can show "日本語の関数".
wx_trunk_15138_20130411_all.patch download (77.7 KB) - added by suzumizaki 12 months ago.
Includes include/*.h, src/*.cpp, and samples/except fixes.
wx_trunk_15138_20130411_core_only.patch download (36.1 KB) - added by suzumizaki 12 months ago.
Only include/*.h and src/*.cpp fixes, included _all version.
wx_trunk_15138_20130411_except_sample_only.patch download (41.6 KB) - added by suzumizaki 12 months ago.
Only samples/except fix, included _all version.
20130417_for_library.patch download (35.9 KB) - added by suzumizaki 12 months ago.
New patch for fix stack tracing codes
20130417_for_samples_except.patch download (41.8 KB) - added by suzumizaki 12 months ago.
New patch for test stack tracing (activated only with Visual C++)

Download all attachments as: .zip

Change History (22)

Changed 13 months ago by suzumizaki

Patchs to trunk to resolve stack tracing problem with non-ASCII identifiers with wxMSW

Changed 13 months ago by suzumizaki

C++ code for test. Please replace minimal.cpp, save as utf-8 or utf-16, build debug mode, and push button and check whether dialog can show "日本語の関数".

comment:1 Changed 13 months ago by vadz

  • Owner set to vadz
  • Status changed from new to accepted

Thanks for working on this. I hoped to apply it quickly but got mired in the differences between debug help API versions and also realized that we're doing some needlessly complicated things with 64 bit support so after spending almost an hour on this I'm still not done :-( Will try to finish at some later time.

In the meanwhile, it would be nice to have a patch to the except sample adding a function with non-ASCII name to be able to check how it works directly here.

TIA!

comment:2 Changed 12 months ago by suzumizaki

Sorry, I found the patch 1st posted seems not working XP and Vista.
I try to make new one, please wait...

comment:3 Changed 12 months ago by vadz

I've already done local modifications, I'll make them available later tonight, either by attaching my patch here or, preferably, by pushing out this branch to a public git repo -- do you use git?

comment:4 Changed 12 months ago by vadz

  • Keywords work-needed added
  • Owner vadz deleted
  • Patch unset
  • Status changed from accepted to new

Sorry, after looking at this one again I have to give up. There are 2 big problems here:

  1. The patch mixes up 32/64 and Unicode-related changes, I'd really like to untangle them.
  2. The patch assumes that we can decide whether we use ANSI or Unicode functions at compile-time which is just not the case as it depends on the version of "dbghelp.dll" we load during run-time. So the code should be ready to work with either ANSI or Unicode functions.

I hoped to fix the former but it doesn't make much sense without fixing the latter and I won't have time to do it. So I've just checked in the 2 small changes that are uncontroversial and will leaving the rest to you...

comment:5 Changed 12 months ago by VZ

(In [73792]) Support Unicode module names in wxDynamicLibrary::MSWGetModuleHandle().

The module names are not necessarily ASCII strings, so use wxString instead of
"char*" and W-version of GetModuleHandle() if appropriate.

See #15138.

comment:6 Changed 12 months ago by suzumizaki

Thank you quick reply and sorry again my late response.
I know the problem you told, and I will try again...

comment:7 Changed 12 months ago by suzumizaki

  • Patch set

I made new patch here now:
1) I tested with samples/except which I also fixes this time. Try the menu "File" - "Show dialog using Unicode named class".
2a) I use Visual C++ 2010 Express for 32bit binary,
2b) and Windows SDK 7.1 for 64bit binary.
3) Both tested Unicode build and not Unicode build.
4) Both tested 32bit .exe and 64bit one.
5) Also Debug and Release build, but partly nonsense with Release one.
6a) Tested with Windows XP Home Edition SP3 32bit version
6b) Windows Vista Home Premium SP2 64bit, with both 32/64bit .exe
6c) Windows 8 Pro 64bit, with both 32/64bit .exe
7) I also test some with Visual Studio Express for Desktop, but only with Windows 8.
8) Above all I used Japanese edition if exists.

...HAHAHA, I have learned a bit how great daily maintainers do their works!

But some light issues remain:
a) For using with older Windows SDKs, we possibly have to define structs which only new SDKs do.
b) With 32bit .exe, SymGetLineFromAddrW64 seems to return FALSE even when the function itself is found. But I added fail safe code, this problem is not critical.
c) While converting wide char to mbcs, last a few characters cut off with some case. But this behavior depends on wxConvLocal.

Thank you!

Changed 12 months ago by suzumizaki

Includes include/*.h, src/*.cpp, and samples/except fixes.

Changed 12 months ago by suzumizaki

Only include/*.h and src/*.cpp fixes, included _all version.

Changed 12 months ago by suzumizaki

Only samples/except fix, included _all version.

comment:8 Changed 12 months ago by suzumizaki

Logically, the patch should be _20130411_all version because samples/except enhancement requires core fix. Last 2 patches are only for convenience.

Changed 12 months ago by suzumizaki

New patch for fix stack tracing codes

Changed 12 months ago by suzumizaki

New patch for test stack tracing (activated only with Visual C++)

comment:9 Changed 12 months ago by suzumizaki

I make new patch to make sure building library without errors with another toolkits.

With this patch, I can build library and samples/except with:
1) TDM_GCC 4.7 included in Code::Blocks 12.11 with mingw.
2) TDM_GCC 4.7 64bit edition (tdm64-gcc-4.7.1.3.exe)
3) i686-w64-mingw32-gcc-4.6.3-2-release-win64_rubenvb.7z
4) (vc90) Windows Server 2008 and .Net FrameWork SDK
5) (vc100) Visual C++ Express 2010 SP1(32bit) & Windows SDK 7.1(64bit)
6) (vc110) Visual Studio Express 2012 for Desktop (of which C++ part)
a) With gcc(1 to 3 above), Debug & Release / Unicode & ANSI build (means 2x2 patterns) succeeded.
b) With VC++(4 to 6, contrary), 32bit & 64bit / Debug & Release / Unicode & ANSI build (means 2x2x2 patterns) succeeded.
c) Without VC++, the new features of samples/except are simply disabled. Just compiled without errors.
d) I built them above on Windows 8 Pro 64bit and run the samples/except.exe-s successfully on Windows XP Home 32bit SP3 and Vista Home Premium 64bit SP2.

Notes, Toolkits I can't build with external reasons:
n1) I can't build with x86_64-w64-mingw32-gcc-4.8.0-win64_rubenvb.7z (mingw-w64 includes gcc 4.8).

The linker shows a lot of FOLDERID_* multi defined issues.

n2) Also I can't with Windows Server 2003 SP1 SDK and R2 one.

Because the toolkit can't compile variadic macros, used in include/wx/cpp.h.
The cl.exe included with this SDK(cl.exe 14.00.40310.41, vc80) seems last version that can't compile variadic macros.

Visual C++ 2005 (cl.exe 14.00.5xxxx.yy, vc80) looks have the capability but I can't install my Windows 8 Pro 64bit.

n3) Also not with more old SDK, like "Microsoft Platform SDK 2002 July"

I can't install my Windows 8 Pro 64bit. The sdk detects 64bit Windows and try to run 64bit installer, but the installer supports itanium only.

I think I did all what I can, please anyone help!

comment:10 Changed 12 months ago by vadz

  • Milestone set to 3.0
  • Owner set to vadz
  • Status changed from new to accepted

Thank you for all your work, I'll try to look at the new patch a.s.a.p.

I probably won't apply the changes to the sample but I'll test with your original test case (stacktestcase.cpp before committing).

comment:11 Changed 10 months ago by vadz

I applied the patch to the latest sources and pushed the result to Github.

Could more people please test this patch please to ensure that it compiles/works correctly with all the different dbghlp.dll versions out there?

Thanks again for your work on this!

comment:12 Changed 7 months ago by vadz

Well, if nobody else has tested it since 3 months, it seems unlikely to happen at all, so I'm going to commit it finally as otherwise the patch is just going to bit rot.

comment:13 Changed 7 months ago by VZ

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

(In [74817]) Make wxMSW stack walking methods work with Unicode identifiers.

This allows to show the stack properly for e.g. Japanese programs.

Closes #15138.

comment:14 Changed 7 months ago by VZ

(In [74820]) Revert "Make wxMSW stack walking methods work with Unicode identifiers."

This reverts r74817 because it broke compilation with VC8 and it doesn't seem
obvious to fix this.

See #15138, closes #15500.

comment:15 Changed 7 months ago by vadz

  • Milestone 3.0 deleted
  • Resolution fixed deleted
  • Status changed from closed to reopened

Unfortunately less than one day of testing in the trunk was enough to find a big problem with this patch: it breaks compilation with VC8 (and certainly VC7 too), see #15000. It would be fine if the Unicode functionality was not available with the older SDKs used by these compilers but it is wrong to completely break compilation with them.

The patch will either have to be reapplied once support for them is officially dropped (not in 3.0 and probably not in 3.2 for VC8) or updated to avoid using PENUMLOADED_MODULES_CALLBACKW64 and other problematic symbols with them.

Note: See TracTickets for help on using tickets.