From 44384d80b32a5bb361c2ec3bee667f7ccee566d7 Mon Sep 17 00:00:00 2001
From: Gabriele Svelto <gsvelto@mozilla.com>
Date: Wed, 30 Jan 2019 09:26:07 +0100
Subject: [PATCH] Always emit a 32-bit crash address for 32-bit architectures

Certain minidumps for 32-bit crashes have the upper 32-bit of the crash
address (which is a 64-bit value) set to non-zero values. This caused a
crash address with more than 32-bits to be printed out for minidumps of
32-bit architectures. This patch masks out those bits when reading the
raw minidump data to ensure this doesn't happen anymore.

Bug: google-breakpad:783

Change-Id: Ieef6dff759fd0ee2efc47c4c4a3cf863a48f0659
Reviewed-on: https://chromium-review.googlesource.com/c/1427819
Reviewed-by: Ted Mielczarek <ted.mielczarek@gmail.com>
---
 src/processor/minidump_processor.cc           |  26 ++++++++++++
 src/processor/minidump_processor_unittest.cc  |  38 ++++++++++++++----
 .../testdata/minidump_32bit_crash_addr.dmp    | Bin 0 -> 11317 bytes
 3 files changed, 56 insertions(+), 8 deletions(-)
 create mode 100644 src/processor/testdata/minidump_32bit_crash_addr.dmp

diff --git a/src/processor/minidump_processor.cc b/src/processor/minidump_processor.cc
index 2b40cf96..e3c3562c 100644
--- a/src/processor/minidump_processor.cc
+++ b/src/processor/minidump_processor.cc
@@ -355,6 +355,26 @@ static const MDRawSystemInfo* GetSystemInfo(Minidump *dump,
   return minidump_system_info->system_info();
 }
 
+static uint64_t GetAddressForArchitecture(const MDCPUArchitecture architecture,
+                                          size_t raw_address)
+{
+  switch (architecture) {
+    case MD_CPU_ARCHITECTURE_X86:
+    case MD_CPU_ARCHITECTURE_MIPS:
+    case MD_CPU_ARCHITECTURE_PPC:
+    case MD_CPU_ARCHITECTURE_SHX:
+    case MD_CPU_ARCHITECTURE_ARM:
+    case MD_CPU_ARCHITECTURE_X86_WIN64:
+      // 32-bit architectures, mask the upper bits.
+      return raw_address & 0xffffffffULL;
+
+    default:
+      // All other architectures either have 64-bit pointers or it's impossible
+      // to tell from the minidump (e.g. MSIL or SPARC) so use 64-bits anyway.
+      return raw_address;
+  }
+}
+
 // Extract CPU info string from ARM-specific MDRawSystemInfo structure.
 // raw_info: pointer to source MDRawSystemInfo.
 // cpu_info: address of target string, cpu info text will be appended to it.
@@ -1637,6 +1657,12 @@ string MinidumpProcessor::GetCrashReason(Minidump *dump, uint64_t *address) {
     }
   }
 
+  if (address) {
+    *address = GetAddressForArchitecture(
+      static_cast<MDCPUArchitecture>(raw_system_info->processor_architecture),
+      *address);
+  }
+
   return reason;
 }
 
diff --git a/src/processor/minidump_processor_unittest.cc b/src/processor/minidump_processor_unittest.cc
index 83682a51..a4ac3685 100644
--- a/src/processor/minidump_processor_unittest.cc
+++ b/src/processor/minidump_processor_unittest.cc
@@ -203,6 +203,12 @@ static const char *kSystemInfoCPUInfo =
 
 #define ASSERT_EQ_ABORT(e1, e2) ASSERT_TRUE_ABORT((e1) == (e2))
 
+static string GetTestDataPath() {
+  char *srcdir = getenv("srcdir");
+
+  return string(srcdir ? srcdir : ".") + "/src/processor/testdata/";
+}
+
 class TestSymbolSupplier : public SymbolSupplier {
  public:
   TestSymbolSupplier() : interrupt_(false) {}
@@ -249,10 +255,8 @@ SymbolSupplier::SymbolResult TestSymbolSupplier::GetSymbolFile(
   }
 
   if (module && module->code_file() == "c:\\test_app.exe") {
-      *symbol_file = string(getenv("srcdir") ? getenv("srcdir") : ".") +
-                     "/src/processor/testdata/symbols/test_app.pdb/" +
-                     module->debug_identifier() +
-                     "/test_app.sym";
+      *symbol_file = GetTestDataPath() + "symbols/test_app.pdb/" +
+                     module->debug_identifier() + "/test_app.sym";
     return FOUND;
   }
 
@@ -460,8 +464,7 @@ TEST_F(MinidumpProcessorTest, TestSymbolSupplierLookupCounts) {
   BasicSourceLineResolver resolver;
   MinidumpProcessor processor(&supplier, &resolver);
 
-  string minidump_file = string(getenv("srcdir") ? getenv("srcdir") : ".") +
-                         "/src/processor/testdata/minidump2.dmp";
+  string minidump_file = GetTestDataPath() + "minidump2.dmp";
   ProcessState state;
   EXPECT_CALL(supplier, GetCStringSymbolData(
       Property(&google_breakpad::CodeModule::code_file,
@@ -501,8 +504,7 @@ TEST_F(MinidumpProcessorTest, TestBasicProcessing) {
   BasicSourceLineResolver resolver;
   MinidumpProcessor processor(&supplier, &resolver);
 
-  string minidump_file = string(getenv("srcdir") ? getenv("srcdir") : ".") +
-                         "/src/processor/testdata/minidump2.dmp";
+  string minidump_file = GetTestDataPath() + "minidump2.dmp";
 
   ProcessState state;
   ASSERT_EQ(processor.Process(minidump_file, &state),
@@ -739,6 +741,26 @@ TEST_F(MinidumpProcessorTest, TestThreadMissingContext) {
   ASSERT_EQ(0U, state.threads()->at(0)->frames()->size());
 }
 
+TEST_F(MinidumpProcessorTest, Test32BitCrashingAddress) {
+  TestSymbolSupplier supplier;
+  BasicSourceLineResolver resolver;
+  MinidumpProcessor processor(&supplier, &resolver);
+
+  string minidump_file = GetTestDataPath() + "minidump_32bit_crash_addr.dmp";
+
+  ProcessState state;
+  ASSERT_EQ(processor.Process(minidump_file, &state),
+            google_breakpad::PROCESS_OK);
+  ASSERT_EQ(state.system_info()->os, kSystemInfoOS);
+  ASSERT_EQ(state.system_info()->os_short, kSystemInfoOSShort);
+  ASSERT_EQ(state.system_info()->os_version, kSystemInfoOSVersion);
+  ASSERT_EQ(state.system_info()->cpu, kSystemInfoCPU);
+  ASSERT_EQ(state.system_info()->cpu_info, kSystemInfoCPUInfo);
+  ASSERT_TRUE(state.crashed());
+  ASSERT_EQ(state.crash_reason(), "EXCEPTION_ACCESS_VIOLATION_WRITE");
+  ASSERT_EQ(state.crash_address(), 0x45U);
+}
+
 }  // namespace
 
 int main(int argc, char *argv[]) {
diff --git a/src/processor/testdata/minidump_32bit_crash_addr.dmp b/src/processor/testdata/minidump_32bit_crash_addr.dmp
new file mode 100644
index 0000000000000000000000000000000000000000..78becc6f1ad8dbb148ca016d6fce604b25df346c
GIT binary patch
literal 11317
zcmeHNeN>d!nSTZm!7mUe*4B@rnN(vH5lI3utFL?*(*-4woH&L=)R_tubc%%H_B~+`
zgdwPzX{>M(Vn}LqvPmqBwqn*8jO@m<YtLaD_ei>Cmt#%T#5Ja&#u%0T{qFm|Fw&H$
z%^x|teK_~M_dehExgYQIUM4F&EBmRPlkblbkvJ5>+NJMg7%h<jXt+?SY$D;Py#qx~
zqm4?EPzK&25`j91avWt4N;1kulxUREC~YV<S$XeJ)Suh&_=^xBk;7~@>V^rKJXj<Q
zbv~5>aj$#zV+E_-_pL5*7k0pC_ZMe=*$to#`k4!Zp}uzw0TKXxt*T)@wxw#SMcoA2
zB4nUY-<%$dLSOfzJdD!a-G&yUZS*Y_6Rn1-kHCS$<`^wsdl=4NDQA1^r?Cok62VIu
zB|@1k!!V!`J)B0P)8IdSp<eYzV?xK>YVha9h(H9Osi(9<37`=3`~+ir6EGPvQ%CkE
z=+_PE5c%wngJsXwn7O-P>y7Y_uQK8`%t}?{<iS=w$5a)LRc3hCt9v4vIsMx58n(1(
z$tsSPs;6I0Ki9`r#Sp_5>;7!r23w{5+FI}v<yrekS#^A*Jaypx)TGF3$F*ayVT*Nt
zwm7b5`nC0*VXHYxP7WI^I}yW@KDMqmh8))${n|QZ*oujg^=(nI3$|XrNn2QUa-v^b
zpBT10;nLn7E*~x2op!m8tuS-GSbtRv2ljiW95rl7l;mbdiPQGI)O)ZuLf7GJbG}&j
zXN%*S)UU0BhAkYD@8|>Nf%=%akB8Xe>1fPX1nL75LTy=R5KJhbtsnGjE2UI1w%f$f
z6sG2@HN=*+udF)M)|c(CqJC}da4K6dgT>(+EKg%?2+pn07+&u>bU@LQ{n~0ZV<=KK
z&L(?dEB&vqwYy(iZyGUd#y)&O<X>S6!@H?*aW5b1$JSWT7l~VnrBn)J1zK^EEz8ha
zg;pY{<55<~-I6OMpsvMpDd-;j#iL(_BA|+-DV~SqK3O2?vQQp^-dymnQ!;KTl9|w#
zE32WwC58BlH*4ekzh3!OuviQ&ZfKy!UM)A`#TrPv`^alIFp6dHy;Q}appTY&Za_;h
zqPPO1DM8M1bK_VEKmHmI9>eS~p4Ue5M#fMC4W;HNZf*<(SR2Ob=#`&+15tFza#<<)
zYDI8W8mmj>Ms$>_D6B;+*I;e+>bQ|L<v}F4GYUfF`^(vwmsvR(h6~_sB@mAsbuaoh
z^<?*JfmzVPGn~cBV-LRU)fewil`U6eRj%nny)9LfG2odOW|J!P$zL9ZM~{ze8{qw}
z<+0_nf&Y3vx2%2h>x`&?z5pfpHxtEytkJP&qRJipa|60~u9Odq@#{xkGv>N+_e2?e
z&aZpG-+HoMUo$(PFGq<)tdB!}4+>A<z7P+c1b)kKTE^qFmdjVlr>|JI(p6kg>Yl!0
zdiuh&2eUFBpD{fvD`P><LNqcLrp?V<ghr0L$O9<_wgf|0nuQn-J=PeOyekq3AL++8
zc`eiT7Ub66yL@^_YrVdD4|8u#s_>n%<s-jttYwin^#8V&t(pGn+FA$yzt?h&8K~~=
zsxSE6YnQ}9WrLVjv2@{lY`PMQecX&^@+|><>n%y$vBDy9>c>-}r=ZS}BaqF%Uz5dI
zIp`DDbrlZ<q35zZY?t5?yR@Tl?P6I2f^r$((4lMjEc(4`+44~_s|U|z+U)f?fb1Xn
zYmeu+n*hDnf4@Erhe_`!0{TZ00ljxDB6*PS9g#8<GOij)x*Us<TBD1Vi-^tC=^S&7
zJPN!;r(`o|wB3rbbb_X6jLNLiDbWG+6J{jN6seKql)Go$^LMl7XmeB3D93or`uFj-
zY7us6SsXL|(E*+Acx0A|$YN&TJ?_G@%x0Hq@<U*fky}lpn53xIa$xR2nFg)|X;@%K
z&yChh^}G{3zWs~v9vAWJ`216MFTTWpegJ0*^^TLSA!U#Y$H!4RW;`3Eb-Y8_d}E(i
zZvsvCzNNkwB0;0fS;XZ$$_JqH?uB&H#zMaX^_eNvI&piJ_;j9OoCi@l4*tZ8reqHg
z!F@>_n$(wzwKZyfl^!)Fpg*uEebM`g4NHnAADmmc`Q2LknZ4a`O;+5UTjE|@vUHip
z6Yp^?7xMjj>HT-6wV%ugTqwR2Ke?=xW>%ND3JZHUa?4Kd-}6vL>W+fiRX_Unz8rF_
zau=_57tT!VVS3FyE5B=A&D^cOJ6DwX;723Kw5HG<!m+gQk&LX-|CI6TfBrOjef|A*
zax8O|F7p(GFcmM^e&G3Wzn}Yy*Q<BGFkr+YG8KDP6qh9RSUT{-4_kN0vA^H-*|zk%
z1J#XWS?LN@lJ(>7pZi7SvzflI(g~BMSALHikF9kVhuAsvY1xkd8MZ35(x13*Ys2Ho
zj84(oQW)y7)42UxQ^(x4Idk8MS4SRu_rp=tQczSBqGavQA50l@;*aw-PcFFiz_uTi
zQi*H%%6xZWZ+P4MgGW~lm_2X(du<O-ePZuwYAJOWuPs=!y2sMz&mVg6x%`f_S0_FZ
zb$fEHhdiFO2#n&uG^<NAUc9kw#{qy!-3F%MQnT;HTi=Vd?#i~awc(Dp(;Zx%Qk(0D
zL>nsyC(vmw$+sfKPk6BIY~|VP+H}WI*Ymuotn+S%fGJ9Lh;j9^HpO{?S{Lr9d;gI`
z{!QEY$t;`hGqlwmjZRW(4_O44!=P-+aYX(${t(PHR-EMxU+sqDSGv1vH+A4yml0W)
z6<#}k=%x)*iS)HVXKllFKK(&Ho16G-X~c6=*3eoXFKz3xq}I1bH7?isTG80(+s|i1
z1D=TA8NdE-w|R9Y1QzfR<AKA?#Y3CHv4F#y)alpbQ;?u#<HFLs<O~Ns2?;~Ny>I)D
zq7FV?sb7R%$~B;$XxgwYQ%?fdJx)`J2ywwnhP~lqHDR03y>3dh>vQ-rMng|iK8Y44
z9?&DTRI-SdawrQDs&z|F5%|Ddg~8!Jd~B?Xbx6<%^Si7U)1M#maiAp*3g}Hs?JC*4
z1pbl>hCf^Jy5b>!KCGc9Cdmu!V&Qq%a2aU8iR^GTK!;iu=rM8iU)?EMM1y+mjRp^O
zfh1l&!0)^8ocR9~D=!uV^o#K0-P{_V7yUH&KR<i}FToYv2R*?F4kg70c^s7a;z~L3
zwD~8Knw+%(hhT$>Dd&je+IrNm+icd6@QMBqV$9m;Yed-?^RK$rAmenD$taX9v(c_y
z^-1IFp9v<{PY3*EJkDeMoO{OMgKr0P-v>J-0UOzd(v8C!D@VM*dEp=9X7#Df0@5Qf
z&y+pp{25+44XV$1C;vs*<j31_%wb=fk>Do}<F#0|DZ|xnT$U|ajx7j1`Bw)h?0=&+
z=r+9Qu9lP2*2<UH2DC(`sW$k3@Jn2_?QlB26VSa#uoV%H-{!&|_l(kSQ+6{{-{L@x
zKKVG;N8xi9+FpF@u&?$Z=%YW273(ADTk#co_p}Pr2L0aQRAWDnk0;2Sh7QiJ>!PtI
zh5Q7_9CkS5z0EtFN-uabq4&<nmy`|*$2t%AlY?Gz7NX`p1YFmeC>j{h{a9~=^kIl4
zAP;MU|3u>2ATR;wK*FO%)Cd!3Yrl}Dp^z^)0CdRlV~}_P@eRO}pM@WU?a*`a4xdUq
zIQ>nhb~*9?7D+(T@Xj*<Jss`kD2!3|nO)9~4|h4Kn`5+da{$<;rx(G~YSL)y=aF2G
zJDf}+NXI>`^6v%cZbYGfY|{sqtvA7BU#)r6$>5<q_FK7>kg$~?sibTn&fC80v*}q)
z@sQJWj|tYCR_HoHyIGpTU&7vLG3}BTk!0YpErflZdmPK985&h79PtE>muLO@+ckw(
zNIXcN5Bx?H!U=~an%O6fb4IypsnBz9{&wJ;CXB+No3k!e*<^pvLyTLPYzN&cj2{c3
z&6J~$cElnRx>(B+$f4bYHhpxXzkOhZUUINrKZTmIU5G&gc>&}j_*Lyy?+Ar3AC_P2
zQeR(~iTUL_3#X|nr%_LZ{qLUe>mx4Js6ErU#;{um{8%X9g#vL;{)@<YoHLS#Jp~%|
zcv7mB%z3;^e}89{dN)NqQ>izDPOxg#!J`p=x1bya{mr}lI{TfFCC-D=jCKd`#PK7g
z?n`)Rmt(VJ^U9w3Gd<+tc}O14fqE|?Vds&+#C@hKpyz|ewaND&d5+Hr=&_(xppb``
zEKK5g@sbRl9PpF34Yj3DWTZ%Ytl&dHcu98YLu2o9Jl0roKD9!Xf^Dgq@`wIspnQ`r
zQ2urqsHW^|fBav)K;e4!VBbnA^4&=_YCoyqWvVkc&N|zz6kG$J1@UHMDCB>dHdmcp
z<f}qka0W9E=iYSy$1=&}=OqI5{%VUR4%p#k0rN9nAW)8UzR#NX1)a7>G`Zy;HS|UH
zKsi6y3EYdJKWf+y8ov{W>&dknHF2VBc}<grk4=egg`Z~^X-YOBrAD$*x`C&6O>yuL
z`3v_wb)z=HAytrj1!o`6xArxf>>TnBJYI$8=P`BeUMqhXc>2Jx^GvhOW=msQj;5Mk
ziSfo=s+Z@E8RyUZ&+1_}8u^AX#!U3@!bO86H)B9R<qzyHJMPy%X)$QD;{mS)IcO)W
zx2tgy$8*vG9ke*;91aBMK=Swq_T)T~ko(zx`1QJHym}+z*?zHJ(b!G~&0B7lxc>?|
zW8k;SbAb1KQ7@gqlZQY&fx1G0_}5i#5sK|?TXMY^OhzAm>deJIB_qIO@%R1rq<|;i
z^g-!D|0w*U?;eb2*}0&zQ~jhUC}S63y#n_}AKZ4}mVO*Wz8O?1Xs-=8%H>_Hm&0f;
zdGlI2-@BHbWV8$M{pvF>iYz_!Apx-~KNNH}wFRBch#mFiBhSjlX3VsOtA7+Lqj+@6
z@Cid~g9eL+4*!b<_=HIsi<tjlW*d_{5HrrA=9T1T6m<B{XMW>Me!!TIBy;Xq2ZkfB
z#d>2td>G$jxeGQZUntQ$AKy%nVVpZL4+*9`X$0cih(~-m-oFG~H}kQf%HX5ogTH6N
z+;Xp|VO=X{_{cb23pU$JKrmLvyN$yAT81&?puGBQK;MY^t%x<{T-$`P*vO=FT=}Rs
z?83$0<$7_TRQ_ks$u?!Y;N6VA9sJz)-2YqQYYXVcHL&q*P1`JOSL&q$a*Qc+iwS78
z45h!ATg0dtH15eR$TR0)9>P9(U&i+&zbTJkzO8xhL=;X!Cb%M?mtZ|88RUGFVea?v
z)C#=@1qP%GbPICe2YGp3xX`Arf*)#8Ki3oEuJ#Dx^h%GMu{!)12Vpeu*^99D>X85A
ztm?*^y)djo51M<~>hn7$6c$6~9>xbY2&0Bq=!^@;L|WXFc3BLa^P&Sf^|Mc3xK;>^
zE&HrP*C6##hXtGm%8-}7kZ$=#9BnOz4%$##0)q6V4C|QwSpGRMsps)=N1k->6W&Jt
zd}@Mmk<0rx^74mm4WPe*^1Bm3C+8rP)&{)gBlRCaf59Jm`KZ^@(+L{az*Db%Io*mE
z|4C2=T`%qqI<57^`3t=hq+WzVyx_CshzorOvTxma^}S!Te%PmYLLSul3T)*eFb&ZC
z=-!}{KLjMu2gZ?5g*|r^^YT-?lS6$>z7KaNpmEO-BGIN#{01e&nWYf*MilOU=ICuG
z$5D8?RzcW>x&?ItYW@J!i;c)S-&}{Rtu}uYngqVvQS&bKGW_L_5lPR(dBh(b9>#aX
z3>5x|kUt8=)XN=H@c?j6`D0F=3ml)%9HT+y84R5%#ytz;W1R`SOJRNuvW@X<1Pw#Q
z4{gkMDHoRoU<!pK#HH-@pp)^kY?Fny8E3}Lg+d6zfAvolDChSGf!`(;?20q?5M}8<
z$H9DKM4Z+$HtHahh3I|tO|Kq@yUw$~(KhD|;p(}k4$YmcAJi6tXFqf?X4F^s9nDyC
wuxEfKybGYemA~=ota++iIG#rm(OBIIJss6^s;P(Pj32zF0#zkIuOTb)zc6OxKL7v#

literal 0
HcmV?d00001