Opened 2 years ago

Closed 5 weeks ago

#15138 closed defect (fixed)

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 (8)

wx_trunk_msw_stacktrace_20130406.patch download (25.2 KB) - added by suzumizaki 2 years ago.
Patchs to trunk to resolve stack tracing problem with non-ASCII identifiers with wxMSW
stacktracetest.cpp download (7.3 KB) - added by suzumizaki 2 years 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 2 years ago.
Includes include/*.h, src/*.cpp, and samples/except fixes.
wx_trunk_15138_20130411_core_only.patch download (36.1 KB) - added by suzumizaki 2 years 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 2 years ago.
Only samples/except fix, included _all version.
20130417_for_library.patch download (35.9 KB) - added by suzumizaki 2 years ago.
New patch for fix stack tracing codes
20130417_for_samples_except.patch download (41.8 KB) - added by suzumizaki 2 years ago.
New patch for test stack tracing (activated only with Visual C++)
except_sample.zip download (190.6 KB) - added by suzumizaki 2 months ago.
New sample code and pictures for reference

Download all attachments as: .zip

Change History (36)

Changed 2 years ago by suzumizaki

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

Changed 2 years 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 2 years 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 2 years 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 2 years 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 2 years 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 2 years 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 2 years 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 2 years 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 2 years ago by suzumizaki

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

Changed 2 years ago by suzumizaki

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

Changed 2 years ago by suzumizaki

Only samples/except fix, included _all version.

comment:8 Changed 2 years 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 2 years ago by suzumizaki

New patch for fix stack tracing codes

Changed 2 years ago by suzumizaki

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

comment:9 Changed 2 years 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 2 years 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 2 years 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 2 years 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 2 years 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 2 years 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 2 years 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.

comment:16 Changed 3 months ago by suzumizaki

Hello, I'm back!

I make the new patch and pull-request on GitHub!
I succeeded to patch against VC8(and VC7 seems automatically
avoid the code by API_VERSION_NUMBER even with the previous
patch).

Please accept this one!

https://github.com/wxWidgets/wxWidgets/pull/38

comment:17 Changed 2 months ago by vadz

  • Status changed from reopened to accepted

I'm looking at this but there is still some work to do, if only to undo the incorrect merge of src/msw/dlmsw.cpp (you seem to have simply overwritten the latest version with your changes), but also other minor things.

I wonder how did you test this exactly with MinGW, do you mean that you could solve #2807 and make this stuff work with it?

comment:18 Changed 2 months ago by suzumizaki

(About 'exact' MinGW)
I don't test with 'exact' MinGW, only Mingw-w64 and TDM-GCC. the TDM-GCC is included codeblocks-13.12mingw-setup.exe. In fact, I know only recently that 'exact' MinGW is different from another two.

(About samples/debugrpt)
I had not known #2807 before you tell me above.
It seems for me some another issue. I tried to fix 'samples/except' 2 years ago, but not 'samples/debugrpt'.

(About current 'master' on GitHub)
Well, continues from http://wxwidgets.blogspot.com/2015/06/build-fixes-for-different-gcc.html , Building with MSVC++ 2005 Express and SDK for Windows Server 2003 SP1 looks fine. Clean and (re)build makes no errors like I reported. The clean build is important I know :) but it takes a lot of time as you know :(

In fact, current 'master' on GitHub cannot be built with UNICODE=0 due to another reason. I will report on wxTrac(as another issue) later.

Now I will trying build with my patch. It may take several hours or days...

Thanks!

Last edited 2 months ago by suzumizaki (previous) (diff)

comment:19 Changed 2 months ago by suzumizaki

I finished to rebuild. see my new report on the GitHub(https://github.com/wxWidgets/wxWidgets/pull/38).

Also, I reported the issue about UNICODE=0, see #17033.

I can't find the dependencies between this issue and #2807.

Thanks and please continue.

comment:20 Changed 2 months ago by vadz

Sorry, I've changed quite a few things locally by now so I'm not sure if I'm going to restart with the new versions of the PR. I'll try to finish the changes today...

comment:21 Changed 2 months ago by suzumizaki

That's no problem, because my part is not changed.
As written in GitHub, I updated to catch up the master branch only.

comment:22 Changed 2 months ago by vadz

I've pushed my version of your patch (with a few other simple fixes) to https://github.com/vadz/wxWidgets/tree/unicode-debughlp, could you please test it and let me know if it still actually works correctly with the non-ASCII symbols?

TIA!

comment:23 Changed 2 months ago by suzumizaki

Ok, I will start test it from now on, please wait...

comment:24 follow-up: Changed 2 months ago by suzumizaki

1) 'DLL Debug' and 'DLL Release' seems always fail with Visual C++

10>     ライブラリ ..\..\lib\vc_x64_dll\wxmsw31u_core.lib とオブジェクト ..\..\lib\vc_x64_dll\wxmsw31u_core.exp を作成中
10>window.obj : error LNK2019: 未解決の外部シンボル "unsigned long __cdecl wxGlobalSEHandler(struct _EXCEPTION_POINTERS *)" (?wxGlobalSEHandler@@YAKPEAU_EXCEPTION_POINTERS@@@Z) が関数 "int `__int64 __cdecl wxWndProc(struct HWND__ *,unsigned int,unsigned __int64,__int64)'::`1'::filt$0" (?filt$0@?0??wxWndProc@@YA_JPEAUHWND__@@I_K_J@Z@4HA) で参照されました。
10>..\..\lib\vc_x64_dll\wxmsw31u_core_vc_x64_custom.dll : fatal error LNK1120: 1 件の未解決の外部参照

2) Definition of 'UNICODE=0' seems always to cause failure with Visual C++

debughlp.cpp
..\..\src\msw\debughlp.cpp(707) : error C2065: 'wxEnumLoaded64Callback' : 定義されていない識別子です。

3) failed with UNICODE=1, VC++ 2005 Express, PSDK for Windows Server 2003 SP1

debughlp.cpp
..\..\src\msw\debughlp.cpp(695) : error C2664: 'BOOL (HANDLE,PENUMLOADED_MODULES_CALLBACKW64,PVOID)' : 2 番目の引数を 'wxPENUMLOADED_MODULES_CALLBACK' から 'PENUMLOADED_MODULES_CALLBACKW64' に変換できません。(新しい機能 ; ヘルプを参照)
        この変換には reinterpret_cast, C スタイル キャストまたは関数スタイルのキャストが必要です。

Wow... Please tell me why your fix is required against my patch...
All these errors don't reported before your fix...
Or did I something wrong?

comment:25 in reply to: ↑ 24 Changed 2 months ago by vadz

Replying to suzumizaki:

1) 'DLL Debug' and 'DLL Release' seems always fail with Visual C++

Thanks, this was broken by an unrelated change, fixed now.

2) Definition of 'UNICODE=0' seems always to cause failure with Visual C++

Oops, sorry, last minute change broke this, fixed too now.

3) failed with UNICODE=1, VC++ 2005 Express, PSDK for Windows Server 2003 SP1

I don't have this compiler any more, but I guess it's due to the first argument non const-ness, right? If so, this should be fixed too now.

Wow... Please tell me why your fix is required against my patch...

As I said, I couldn't apply the patch without changes because of the bad merge of src/msw/dlmsw.cpp (you basically threw away all the changes to this file since the last 2 years instead of really merging them). And as long as I was looking at the patch anyhow, I decided to simplify it because, as I hope you agree, the old code with all of its copying between SYMBOL_INFO and SYMBOL_INFOW was very messy and the new version is much cleaner. It's unfortunate that some things have broken in the process but it's not the worst thing in the world (as long as they're fixed).

Could you please retest the new version in the same branch as in the commment:22? Not necessarily all the compilation cases (although testing with VC8 would be welcome), but whether the code actually works with Japanese (or other non ASCII) identifiers? I don't have any test code for this and this is what worries me the most.

TIA!

Changed 2 months ago by suzumizaki

New sample code and pictures for reference

comment:26 Changed 2 months ago by suzumizaki

Ok I know, thank you to take your time to explain.

but whether the code actually works with Japanese (or other non ASCII) identifiers?
I don't have any test code for this and this is what worries me the most.

If you have a time, please check new attachment zip posted just now.
This is almost same as 20130417_for_samples_except.patch with comment 8,
but all identifiers are replaced with '\uXXXX' form.
And I append the pictures, how the dialogs should be shown.

In fact, encoding of source codes are optional even in C++11(also even with UTF-8),
but all compilers should accept \uXXXX form that meets C++98/03/11 or later.

Could you please retest the new version in the same branch as in the commment:22?

I'm testing now... Currently, all of below seems fine.

  • Visual Studio 2013 Community build with .sln file
    • means 8 patterns Debug/Release, DLL Debug/DLL Release, Win32/x64
    • (all implied UNICODE=1, UNICODE=0 is not tested)
  • Visual C++ 2005 Express with PSDK for Server 2003 SP1
    • Debug Win32/Release Win32 with wx_vc8.sln
    • Debug Win32/Release Win32 with UNICODE=0 with nmake on command prompt
    • wx_vc8.sln file doesn't have the dependencies information for DLL Debug/DLL Release builds, but repeatedly rebuilding all projects will make success.

comment:27 Changed 2 months ago by suzumizaki

And following configures succeeded to build with no errors:

  • Visual Studio 2013 Community, with nmake on command prompt
    • DLL Debug x86 UNICODE=0/DLL Debug x64 UNICODE=0
  • TDM-GCC included in Code::Blocks 13.12
    • Debug/Release
  • MinGW-w64(x86_64-5.1.0-win32-seh-rt_v4-rev0)
    • Debug/Release
  • MinGW "classic"
    • Debug/Release
    • ...but only with "classic", I don't know how to build the application.

comment:28 Changed 5 weeks ago by Vadim Zeitlin <vadim@…>

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

In 28587c97d88fbf9b84af6c7151347b2c409268a6/git-wxWidgets:

Add support for Unicode to wxStackWalker.

Use wide-char versions of debug help functions if available, falling back to
the narrow char ones otherwise.

Also improve 64 bit support by using 64 bit versions of the functions if
available as well.

Closes #15138.

Note: See TracTickets for help on using tickets.