Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(194)

Issue 2024003: Add ExceptionBarrier to module scanning in the hope of reducing some false po... (Closed)

Created:
10 years, 7 months ago by robertshield
Modified:
9 years, 6 months ago
CC:
chromium-reviews, amit
Visibility:
Public.

Description

Add ExceptionBarrier to module scanning in the hope of reducing some false positives. BUG=43343 Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=46683

Patch Set 1 #

Total comments: 6

Patch Set 2 : '' #

Unified diffs Side-by-side diffs Delta from patch set Stats (+22 lines, -10 lines) Patch
M chrome_frame/module_utils.cc View 1 2 chunks +22 lines, -10 lines 0 comments Download

Messages

Total messages: 3 (0 generated)
robertshield
10 years, 7 months ago (2010-05-06 21:43:35 UTC) #1
tommi (sloooow) - chröme
http://codereview.chromium.org/2024003/diff/1/2 File chrome_frame/module_utils.cc (right): http://codereview.chromium.org/2024003/diff/1/2#newcode80 chrome_frame/module_utils.cc:80: ExceptionBarrier exception_barrier; would it make sense to keep the ...
10 years, 7 months ago (2010-05-06 21:47:42 UTC) #2
robertshield
10 years, 7 months ago (2010-05-07 13:42:55 UTC) #3
Done, thanks. TBRing for now, to make it easier to do the rest of the work.

http://codereview.chromium.org/2024003/diff/1/2
File chrome_frame/module_utils.cc (right):

http://codereview.chromium.org/2024003/diff/1/2#newcode80
chrome_frame/module_utils.cc:80: ExceptionBarrier exception_barrier;
On 2010/05/06 21:47:43, tommi wrote:
> would it make sense to keep the scope of the exception barrier smaller?  I.e.
> only for the outgoing calls?

Yeah, it would let us know if there are other patched api calls that our veh
will pick up on. Done.

http://codereview.chromium.org/2024003/diff/1/2#newcode87
chrome_frame/module_utils.cc:87: MODULEENTRY32W module_entry;
On 2010/05/06 21:47:43, tommi wrote:
> suggest we also zero out this struct.

Done, sorry I lost this during some svn tomfoolery.

http://codereview.chromium.org/2024003/diff/1/2#newcode95
chrome_frame/module_utils.cc:95: cont = Module32NextW(snapshot, &module_entry);
On 2010/05/06 21:47:43, tommi wrote:
> also initialize and zero out this one prior to this call.

Done.

Powered by Google App Engine
This is Rietveld 408576698