Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.

Add missing exception holder to EX_TRY #1878

Merged
merged 1 commit into from
Oct 27, 2015
Merged

Add missing exception holder to EX_TRY #1878

merged 1 commit into from
Oct 27, 2015

Conversation

janvorli
Copy link
Member

When the exception handling was changed recently, the exception holder
was added only to PAL_TRY / PAL_EXCEPT macros. But it needs to be
in the EX_TRY as well, otherwise first pass of exception handling
would incorrectly consider some exceptions unhandled.

@janvorli
Copy link
Member Author

@mikem8361 can you take a look please?
cc: @Djuffin

mikem8361
mikem8361 reviewed Oct 27, 2015
View reviewed changes
src/inc/ex.h
#define EX_TRY_HOLDER \
auto __exceptionHolder = NativeExceptionHolderFactory::CreateHolder(&__defaultExceptionFilter); \
__exceptionHolder.Push();

Copy link
Member

Choose a reason for hiding this comment

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

This can be simplified to use another non-templated derived class of NativeExceptionHolderBase that overrides InvokeFilter to return EXCEPTION_EXECUTE_HANDLER.

class NativeExceptionHolderCatchAll : public NativeExceptionHolderBase
{

public:
    NativeExceptionHolderCatchAll()
        : NativeExceptionHolderBase()
    {
        Push();
    }

    virtual EXCEPTION_DISPOSITION InvokeFilter(PAL_SEHException& ex)
    {
        return EXCEPTION_EXECUTE_HANDLER;
    }
};

So then the EX_TRY_HOLDER can be just:
auto NativeExceptionHolderCatchAll __exceptionHolder;

@janvorli
Copy link
Member Author

@mikem8361 thanks, it is a good idea. I'll change it that way.

@janvorli
Add missing exception holder to EX_TRY
a06bea2 
When the exception handling was changed recently, the exception holder
was added only to PAL_TRY / PAL_EXCEPT macros. But it needs to be
in the EX_TRY as well, otherwise first pass of exception handling
would incorrectly consider some exceptions unhandled.
@janvorli
Copy link
Member Author

@mikem8361 can you take another look please? I've modified it based on your suggestion.

@mikem8361
Copy link
Member

LGTM. Thanks.

@Djuffin
Copy link

Djuffin commented Oct 27, 2015

I validated that this fix really addressed funeval crash issue. LGTM.

janvorli added a commit that referenced this pull request Oct 27, 2015
@janvorli
Merge pull request #1878 from janvorli/fix-ex-try
7a08037 
Add missing exception holder to EX_TRY
@janvorli janvorli merged commit 7a08037 into dotnet:master Oct 27, 2015
@janvorli janvorli deleted the fix-ex-try branch October 27, 2015 23:10
sdmaclea added a commit to sdmaclea/coreclr that referenced this pull request Apr 14, 2020
@sdmaclea
Port Cross OS DAC 5.0 work to 3.1
7e78da7 
 + Auto renames for (dotnet/coreclr/dotnet#26080)
 + Rename _WIN64 define to BIT64 (dotnet#26080)
 + Remove stray sos reference (dotnet/runtime/dotnet#1875)
 + Fix MSVC warn as errors... (dotnet/runtime/dotnet#1876)
 + Remove unused/unnecessary defines (dotnet/runtime/dotnet#1878)
 + Rename CLR_CMAKE_PLATFORM* CLR_CMAKE_HOST* (dotnet/runtime/#1974)
 + Rename PAL_CMAKE_* CLR_CMAKE_* (dotnet/runtime/dotnet#1984)
 + Add alpinedac & linuxdac build options (dotnet/runtime/dotnet#2056)
 + Refactor CMake system to allow cross OS DAC compile (dotnet/runtime/#2054)
 + Add FEATURE_REMOTE_PROC_MEM (dotnet/runtime/dotnet#2244)
 + Fix compilation of dbgtransportsession (dotnet/runtime/dotnet#2247)
 + Auto renames from (dotnet/runtime/#2256)
 + Rename cross compilation related defines (dotnet/runtime/#2256)
 + Fix logic to disable mscordbi build (dotnet/runtime/#31745)
 + Fix unused variable warning (dotnet/runtime/#31747)
 + Remove GetRecycleMemoryInfo from DAC builds (dotnet/runtime/#31748)
 + Fix check.* cross compilation linker errors (dotnet/runtime/#31751)
 + Reduce PAL DAC exports (dotnet/runtime/#31749)
 + Add EMPTY_BASES_DECL (dotnet/runtime/dotnet#26980)
 + Disable linux unwinder on Windows (dotnet/runtime/#31777)
 + Add cross OS support for MAKEDLLNAME (dotnet/runtime/#31746)
 + Fix host compiler when cross OS DAC compiling (dotnet/runtime/#31775)
 + Fix arm64singlestepper #ifdef (dotnet/runtime/#31776)
 + Configure host features based on host OS (dotnet/runtime/#31778)
 + Configure Host OS APIs based on HOST macros (dotnet/runtime/#31774)
 + Use common CLR_CMAKE_* variables in more places (dotnet/runtime/#31659)
 + Add T_CRITICAL_SECTION for cross OS DAC compile (dotnet/runtime/#31908)
 + Fix arm cross OS DAC compilation (dotnet/runtime/#31903)
 + Add CrossOS DAC build back (dotnet/runtime/#33064)
 + Fix typo (dotnet/runtime/#33192)
 + Fix type layout whan Cross OS compiling (dotnet/runtime/#33487)
 + Partial revert auto renames from (dotnet/runtime/dotnet#2056)
 + Partial revert (dotnet/runtime/dotnet#26080)
sdmaclea added a commit to sdmaclea/coreclr that referenced this pull request Jun 25, 2020
@sdmaclea
Port Cross OS DAC 5.0 work to 3.1
1a42a02 
 + Auto renames for (dotnet/coreclr/dotnet#26080)
 + Rename _WIN64 define to BIT64 (dotnet#26080)
 + Remove stray sos reference (dotnet/runtime/dotnet#1875)
 + Fix MSVC warn as errors... (dotnet/runtime/dotnet#1876)
 + Remove unused/unnecessary defines (dotnet/runtime/dotnet#1878)
 + Rename CLR_CMAKE_PLATFORM* CLR_CMAKE_HOST* (dotnet/runtime/#1974)
 + Rename PAL_CMAKE_* CLR_CMAKE_* (dotnet/runtime/dotnet#1984)
 + Add alpinedac & linuxdac build options (dotnet/runtime/dotnet#2056)
 + Refactor CMake system to allow cross OS DAC compile (dotnet/runtime/#2054)
 + Add FEATURE_REMOTE_PROC_MEM (dotnet/runtime/dotnet#2244)
 + Fix compilation of dbgtransportsession (dotnet/runtime/dotnet#2247)
 + Auto renames from (dotnet/runtime/#2256)
 + Rename cross compilation related defines (dotnet/runtime/#2256)
 + Fix logic to disable mscordbi build (dotnet/runtime/#31745)
 + Fix unused variable warning (dotnet/runtime/#31747)
 + Remove GetRecycleMemoryInfo from DAC builds (dotnet/runtime/#31748)
 + Fix check.* cross compilation linker errors (dotnet/runtime/#31751)
 + Reduce PAL DAC exports (dotnet/runtime/#31749)
 + Add EMPTY_BASES_DECL (dotnet/runtime/dotnet#26980)
 + Disable linux unwinder on Windows (dotnet/runtime/#31777)
 + Add cross OS support for MAKEDLLNAME (dotnet/runtime/#31746)
 + Fix host compiler when cross OS DAC compiling (dotnet/runtime/#31775)
 + Fix arm64singlestepper #ifdef (dotnet/runtime/#31776)
 + Configure host features based on host OS (dotnet/runtime/#31778)
 + Configure Host OS APIs based on HOST macros (dotnet/runtime/#31774)
 + Use common CLR_CMAKE_* variables in more places (dotnet/runtime/#31659)
 + Add T_CRITICAL_SECTION for cross OS DAC compile (dotnet/runtime/#31908)
 + Fix arm cross OS DAC compilation (dotnet/runtime/#31903)
 + Add CrossOS DAC build back (dotnet/runtime/#33064)
 + Fix typo (dotnet/runtime/#33192)
 + Fix type layout whan Cross OS compiling (dotnet/runtime/#33487)
 + Partial revert auto renames from (dotnet/runtime/dotnet#2056)
 + Partial revert (dotnet/runtime/dotnet#26080)
sdmaclea added a commit to sdmaclea/coreclr that referenced this pull request Jun 25, 2020
@sdmaclea
Port Cross OS DAC 5.0 work to 3.1
f6ebecb 
 + Auto renames for (dotnet/coreclr/dotnet#26080)
 + Rename _WIN64 define to BIT64 (dotnet#26080)
 + Remove stray sos reference (dotnet/runtime/dotnet#1875)
 + Fix MSVC warn as errors... (dotnet/runtime/dotnet#1876)
 + Remove unused/unnecessary defines (dotnet/runtime/dotnet#1878)
 + Rename CLR_CMAKE_PLATFORM* CLR_CMAKE_HOST* (dotnet/runtime/#1974)
 + Rename PAL_CMAKE_* CLR_CMAKE_* (dotnet/runtime/dotnet#1984)
 + Add alpinedac & linuxdac build options (dotnet/runtime/dotnet#2056)
 + Refactor CMake system to allow cross OS DAC compile (dotnet/runtime/#2054)
 + Add FEATURE_REMOTE_PROC_MEM (dotnet/runtime/dotnet#2244)
 + Fix compilation of dbgtransportsession (dotnet/runtime/dotnet#2247)
 + Auto renames from (dotnet/runtime/#2256)
 + Rename cross compilation related defines (dotnet/runtime/#2256)
 + Fix logic to disable mscordbi build (dotnet/runtime/#31745)
 + Fix unused variable warning (dotnet/runtime/#31747)
 + Remove GetRecycleMemoryInfo from DAC builds (dotnet/runtime/#31748)
 + Fix check.* cross compilation linker errors (dotnet/runtime/#31751)
 + Reduce PAL DAC exports (dotnet/runtime/#31749)
 + Add EMPTY_BASES_DECL (dotnet/runtime/dotnet#26980)
 + Disable linux unwinder on Windows (dotnet/runtime/#31777)
 + Add cross OS support for MAKEDLLNAME (dotnet/runtime/#31746)
 + Fix host compiler when cross OS DAC compiling (dotnet/runtime/#31775)
 + Fix arm64singlestepper #ifdef (dotnet/runtime/#31776)
 + Configure host features based on host OS (dotnet/runtime/#31778)
 + Configure Host OS APIs based on HOST macros (dotnet/runtime/#31774)
 + Use common CLR_CMAKE_* variables in more places (dotnet/runtime/#31659)
 + Add T_CRITICAL_SECTION for cross OS DAC compile (dotnet/runtime/#31908)
 + Fix arm cross OS DAC compilation (dotnet/runtime/#31903)
 + Add CrossOS DAC build back (dotnet/runtime/#33064)
 + Fix typo (dotnet/runtime/#33192)
 + Fix type layout whan Cross OS compiling (dotnet/runtime/#33487)
 + Partial revert auto renames from (dotnet/runtime/dotnet#2056)
 + Partial revert (dotnet/runtime/dotnet#26080)
 + Enable cross OS DBI build(dotnet/runtime/#35021)
sdmaclea added a commit to sdmaclea/coreclr that referenced this pull request Jul 1, 2020
@sdmaclea
Port Cross OS DAC 5.0 work to 3.1
16fe147 
 + Auto renames for (dotnet/coreclr/dotnet#26080)
 + Rename _WIN64 define to BIT64 (dotnet#26080)
 + Remove stray sos reference (dotnet/runtime/dotnet#1875)
 + Fix MSVC warn as errors... (dotnet/runtime/dotnet#1876)
 + Remove unused/unnecessary defines (dotnet/runtime/dotnet#1878)
 + Rename CLR_CMAKE_PLATFORM* CLR_CMAKE_HOST* (dotnet/runtime/#1974)
 + Rename PAL_CMAKE_* CLR_CMAKE_* (dotnet/runtime/dotnet#1984)
 + Add alpinedac & linuxdac build options (dotnet/runtime/dotnet#2056)
 + Refactor CMake system to allow cross OS DAC compile (dotnet/runtime/#2054)
 + Add FEATURE_REMOTE_PROC_MEM (dotnet/runtime/dotnet#2244)
 + Fix compilation of dbgtransportsession (dotnet/runtime/dotnet#2247)
 + Auto renames from (dotnet/runtime/#2256)
 + Rename cross compilation related defines (dotnet/runtime/#2256)
 + Fix logic to disable mscordbi build (dotnet/runtime/#31745)
 + Fix unused variable warning (dotnet/runtime/#31747)
 + Remove GetRecycleMemoryInfo from DAC builds (dotnet/runtime/#31748)
 + Fix check.* cross compilation linker errors (dotnet/runtime/#31751)
 + Reduce PAL DAC exports (dotnet/runtime/#31749)
 + Add EMPTY_BASES_DECL (dotnet/runtime/dotnet#26980)
 + Disable linux unwinder on Windows (dotnet/runtime/#31777)
 + Add cross OS support for MAKEDLLNAME (dotnet/runtime/#31746)
 + Fix host compiler when cross OS DAC compiling (dotnet/runtime/#31775)
 + Fix arm64singlestepper #ifdef (dotnet/runtime/#31776)
 + Configure host features based on host OS (dotnet/runtime/#31778)
 + Configure Host OS APIs based on HOST macros (dotnet/runtime/#31774)
 + Use common CLR_CMAKE_* variables in more places (dotnet/runtime/#31659)
 + Add T_CRITICAL_SECTION for cross OS DAC compile (dotnet/runtime/#31908)
 + Fix arm cross OS DAC compilation (dotnet/runtime/#31903)
 + Add CrossOS DAC build back (dotnet/runtime/#33064)
 + Fix typo (dotnet/runtime/#33192)
 + Fix type layout whan Cross OS compiling (dotnet/runtime/#33487)
 + Partial revert auto renames from (dotnet/runtime/dotnet#2056)
 + Partial revert (dotnet/runtime/dotnet#26080)
 + Enable cross OS DBI build(dotnet/runtime/#35021)
 + Disable EMPTY_BASE_DECL except for cross OS DAC
 + Fix DBI type layout issues
   + Add utilcode_dbi
   + Consistently define DBI features
   + Define T_CRITICAL_SECTION during non-DAC builds
 + Port cross DAC unwind support to 3.1
   + Libunwind 1.5rc2 again (dotnet/runtime#36988)
 + Add libunwind to cross DAC (dotnet/runtime#37521)
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
@janvorli
Merge pull request dotnet/coreclr#1878 from janvorli/fix-ex-try
9ed9ea7 
Add missing exception holder to EX_TRY

Commit migrated from dotnet/coreclr@7a08037
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@janvorli @mikem8361 @Djuffin @dotnet-bot @dnfclas