From patchwork Mon Apr 6 16:11:33 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Giuliano Procida X-Patchwork-Id: 39080 From: gprocida@google.com (Giuliano Procida) Date: Mon, 6 Apr 2020 17:11:33 +0100 Subject: [PATCH RFC] Allow variable suppression to be refined by scope. Message-ID: <20200406161132.28555-1-gprocida@google.com> Variable suppression is predicated on one or more of the variable's name, type and location. However, there is no distinction between the scopes a variable might exist in: char pad[2]; struct S { char pad[2]; }; This patch adds the ability to filter based on scope. To target the global scope explicity, scope_name_regex should be set to "^$". Signed-off-by: Giuliano Procida --- include/abg-suppression.h | 16 ++- src/abg-suppression-priv.h | 34 +++++- src/abg-suppression.cc | 107 +++++++++++++++++- tests/data/Makefile.am | 6 + .../test49-var-scope-report.txt | 51 +++++++++ .../test49-var-scope-suppr.txt | 3 + .../test-diff-suppr/test49-var-scope-v0.cc | 30 +++++ .../test-diff-suppr/test49-var-scope-v0.o | Bin 0 -> 4136 bytes .../test-diff-suppr/test49-var-scope-v1.cc | 32 ++++++ .../test-diff-suppr/test49-var-scope-v1.o | Bin 0 -> 4232 bytes tests/test-diff-suppr.cc | 10 ++ 11 files changed, 282 insertions(+), 7 deletions(-) create mode 100644 tests/data/test-diff-suppr/test49-var-scope-report.txt create mode 100644 tests/data/test-diff-suppr/test49-var-scope-suppr.txt create mode 100644 tests/data/test-diff-suppr/test49-var-scope-v0.cc create mode 100644 tests/data/test-diff-suppr/test49-var-scope-v0.o create mode 100644 tests/data/test-diff-suppr/test49-var-scope-v1.cc create mode 100644 tests/data/test-diff-suppr/test49-var-scope-v1.o diff --git a/include/abg-suppression.h b/include/abg-suppression.h index 4f1fb417..bdca38a5 100644 --- a/include/abg-suppression.h +++ b/include/abg-suppression.h @@ -685,7 +685,9 @@ public: const string& symbol_version = "", const string& symbol_version_regex_str = "", const string& type_name = "", - const string& type_name_regex_str = ""); + const string& type_name_regex_str = "", + const string& scope_name = "", + const string& scope_name_regex_str = ""); virtual ~variable_suppression(); @@ -758,6 +760,18 @@ public: void set_type_name_regex_str(const string&); + const string& + get_scope_name() const; + + void + set_scope_name(const string&); + + const string& + get_scope_name_regex_str() const; + + void + set_scope_name_regex_str(const string&); + bool suppresses_diff(const diff* d) const; diff --git a/src/abg-suppression-priv.h b/src/abg-suppression-priv.h index 71057ce1..9b80047d 100644 --- a/src/abg-suppression-priv.h +++ b/src/abg-suppression-priv.h @@ -575,6 +575,9 @@ struct variable_suppression::priv string type_name_; string type_name_regex_str_; mutable sptr_utils::regex_t_sptr type_name_regex_; + string scope_name_; + string scope_name_regex_str_; + mutable sptr_utils::regex_t_sptr scope_name_regex_; priv(const string& name, const string& name_regex_str, @@ -583,7 +586,9 @@ struct variable_suppression::priv const string& symbol_version, const string& symbol_version_regex_str, const string& type_name, - const string& type_name_regex_str) + const string& type_name_regex_str, + const string& scope_name, + const string& scope_name_regex_str) : change_kind_(ALL_CHANGE_KIND), name_(name), name_regex_str_(name_regex_str), @@ -592,7 +597,9 @@ struct variable_suppression::priv symbol_version_(symbol_version), symbol_version_regex_str_(symbol_version_regex_str), type_name_(type_name), - type_name_regex_str_(type_name_regex_str) + type_name_regex_str_(type_name_regex_str), + scope_name_(scope_name), + scope_name_regex_str_(scope_name_regex_str) {} /// Getter for a pointer to a regular expression object built from @@ -731,6 +738,29 @@ struct variable_suppression::priv } return type_name_regex_; } + + /// Getter for a pointer to a regular expression object built from + /// the regular expression string + /// variable_suppression::priv::scope_name_regex_str_. + /// + /// If that string is empty, then an empty regular expression object + /// pointer is returned. + /// + /// @return a pointer to the regular expression object of + /// variable_suppression::priv::scope_name_regex_str_. + const sptr_utils::regex_t_sptr + get_scope_name_regex() const + { + if (!scope_name_regex_ && !scope_name_regex_str_.empty()) + { + sptr_utils::regex_t_sptr r = sptr_utils::build_sptr(); + if (regcomp(r.get(), + scope_name_regex_str_.c_str(), + REG_EXTENDED) == 0) + scope_name_regex_ = r; + } + return scope_name_regex_; + } };// end class variable_supppression::priv template diff --git a/src/abg-suppression.cc b/src/abg-suppression.cc index d3ccb63c..f4651d01 100644 --- a/src/abg-suppression.cc +++ b/src/abg-suppression.cc @@ -3412,6 +3412,18 @@ read_function_suppression(const ini::config::section& section) /// If @p type_name is not empty, then this parameter is ignored at /// evluation time. This parameter might be empty, in which case it's /// ignored at evaluation time. +/// +/// @param scope_name the name of the scope (class or union) of the +/// variable the user wants the current specification to designate. +/// This parameter might be empty, in which case it's ignored at +/// evaluation time and global scope is assumed. +/// +/// @param scope_name_regex_str if @p scope_name is empty, then this +/// parameter is a regular expression for a family of scope (class or +/// union) names of variables the user wants the current specification +/// to designate. If this parameter is empty (or @p scope_name is not +/// empty) then it's ignored at evaluation time and global scope is +/// assumed. variable_suppression::variable_suppression(const string& label, const string& name, const string& name_regex_str, @@ -3420,12 +3432,15 @@ variable_suppression::variable_suppression(const string& label, const string& symbol_version, const string& symbol_version_regex, const string& type_name, - const string& type_name_regex_str) + const string& type_name_regex_str, + const string& scope_name, + const string& scope_name_regex_str) : suppression_base(label), priv_(new priv(name, name_regex_str, symbol_name, symbol_name_regex_str, symbol_version, symbol_version_regex, - type_name, type_name_regex_str)) + type_name, type_name_regex_str, + scope_name, scope_name_regex_str)) {} /// Virtual destructor for the @erf variable_suppression type. @@ -3702,6 +3717,54 @@ void variable_suppression::set_type_name_regex_str(const string& r) {priv_->type_name_regex_str_ = r;} +/// Getter for the name of the scope of the variable the user wants the +/// current specification to designate. +/// +/// This property might be empty, in which case it's ignored at +/// evaluation time. +/// +/// @return the name of the variable scope. +const string& +variable_suppression::get_scope_name() const +{return priv_->scope_name_;} + +/// Setter for the name of the scope of the variable the user wants the +/// current specification to designate. +/// +/// This property might be empty, in which case it's ignored at +/// evaluation time. +/// +/// @param n the new name of the variable scope. +void +variable_suppression::set_scope_name(const string& n) +{priv_->scope_name_ = n;} + +/// Getter for the regular expression for a family of scope names of +/// variables the user wants the current specification to designate. +/// +/// If the scope name as returned by +/// variable_suppression::get_scope_name() is not empty, then this +/// property is ignored at evaluation time. This property might be +/// empty, in which case it's ignored at evaluation time. +/// +/// @return the regular expression of the variable scope name. +const string& +variable_suppression::get_scope_name_regex_str() const +{return priv_->scope_name_regex_str_;} + +/// Setter for the regular expression for a family of scope names of +/// variables the user wants the current specification to designate. +/// +/// If the scope name as returned by +/// variable_suppression::get_scope_name() is not empty, then this +/// property is ignored at evaluation time. This property might be +/// empty, in which case it's ignored at evaluation time. +/// +/// @param r the regular expression of the variable scope name. +void +variable_suppression::set_scope_name_regex_str(const string& r) +{priv_->scope_name_regex_str_ = r;} + /// Evaluate this suppression specification on a given diff node and /// say if the diff node should be suppressed or not. /// @@ -3861,6 +3924,27 @@ variable_suppression::suppresses_variable(const var_decl* var, } } + // Check for the scope_name and scope_name_regex properties match. + std::string scope_name = var->get_scope()->get_name(); + const std::string& scope_name_is = get_scope_name(); + const sptr_utils::regex_t_sptr scope_name_regex = + priv_->get_scope_name_regex(); + // std::cerr << "var='" << var->get_qualified_name() << "' scope='" << scope_name << "' exact='" << scope_name_is << "' regex='" << priv_->scope_name_regex_str_ << "'" << std::endl; + if (!scope_name_is.empty()) + { + // scope string match + // std::cerr << "matching str " << scope_name_is << ", " << scope_name << "std::endl"; + if (scope_name_is != scope_name) + return false; + } + else if (scope_name_regex) + { + // scope regex match + // std::cerr << "matching regex " << priv_->scope_name_regex_str_ << ", " << scope_name << "std::endl"; + if (regexec(scope_name_regex.get(), scope_name.c_str(), 0, NULL, 0) != 0) + return false; + } + return true; } @@ -4165,6 +4249,18 @@ read_variable_suppression(const ini::config::section& section) ? type_name_regex_prop->get_value()->as_string() : ""; + ini::simple_property_sptr scope_name_prop = + is_simple_property(section.find_property("scope_name")); + string scope_name_str = scope_name_prop + ? scope_name_prop->get_value()->as_string() + : ""; + + ini::simple_property_sptr scope_name_regex_prop = + is_simple_property(section.find_property("scope_name_regexp")); + string scope_name_regex_str = scope_name_regex_prop + ? scope_name_regex_prop->get_value()->as_string() + : ""; + if (label_str.empty() && name_str.empty() && name_regex_str.empty() @@ -4179,13 +4275,16 @@ read_variable_suppression(const ini::config::section& section) && symbol_version.empty() && symbol_version_regex_str.empty() && type_name_str.empty() - && type_name_regex_str.empty()) + && type_name_regex_str.empty() + && scope_name_str.empty() + && scope_name_regex_str.empty()) return result; result.reset(new variable_suppression(label_str, name_str, name_regex_str, symbol_name, symbol_name_regex_str, symbol_version, symbol_version_regex_str, - type_name_str, type_name_regex_str)); + type_name_str, type_name_regex_str, + scope_name_str, scope_name_regex_str)); if ((drop_artifact_str == "yes" || drop_artifact_str == "true") && (!name_str.empty() diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am index bb0fd83b..5da4510e 100644 --- a/tests/data/Makefile.am +++ b/tests/data/Makefile.am @@ -1318,6 +1318,12 @@ test-diff-suppr/libtest48-soname-abixml-suppr.txt \ test-diff-suppr/libtest48-soname-abixml-suppr-2.txt \ test-diff-suppr/libtest48-soname-abixml-suppr-3.txt \ test-diff-suppr/libtest48-soname-abixml-suppr-4.txt \ +test-diff-suppr/test49-var-scope-report.txt \ +test-diff-suppr/test49-var-scope-suppr.txt \ +test-diff-suppr/test49-var-scope-v0.cc \ +test-diff-suppr/test49-var-scope-v0.o \ +test-diff-suppr/test49-var-scope-v1.cc \ +test-diff-suppr/test49-var-scope-v1.o \ \ test-diff-dwarf-abixml/test0-pr19026-libvtkIOSQL-6.1.so.1 \ test-diff-dwarf-abixml/test0-pr19026-libvtkIOSQL-6.1.so.1.abi \ diff --git a/tests/data/test-diff-suppr/test49-var-scope-report.txt b/tests/data/test-diff-suppr/test49-var-scope-report.txt new file mode 100644 index 00000000..c4211296 --- /dev/null +++ b/tests/data/test-diff-suppr/test49-var-scope-report.txt @@ -0,0 +1,51 @@ +Functions changes summary: 0 Removed, 4 Changed, 0 Added functions +Variables changes summary: 0 Removed, 1 Changed (1 filtered out), 0 Added variables + +4 functions with some indirect sub-type change: + + [C] 'function void reg(S*)' at test49-var-scope-v1.cc:29:1 has some indirect sub-type changes: + parameter 1 of type 'S*' has sub-type changes: + in pointed to type 'struct S' at test49-var-scope-v1.cc:1:1: + type size hasn't changed + 1 data member insertion: + 'char S::wanted[10]', at offset 0 (in bits) at test49-var-scope-v1.cc:2:1 + 1 data member change: + type of 'char S::padding[20]' changed: + type name changed from 'char[20]' to 'char[10]' + array type size changed from 160 to 80 + array type subrange 1 changed length from 20 to 10 + and offset changed from 0 to 80 (in bits) (by +80 bits) + + [C] 'function void reg(U*)' at test49-var-scope-v1.cc:31:1 has some indirect sub-type changes: + parameter 1 of type 'U*' has sub-type changes: + in pointed to type 'union U' at test49-var-scope-v1.cc:11:1: + type size changed from 160 to 256 (in bits) + 1 data member change: + type of 'char U::padding[20]' changed: + type name changed from 'char[20]' to 'char[32]' + array type size changed from 160 to 256 + array type subrange 1 changed length from 20 to 32 + + [C] 'function void reg(Sm*)' at test49-var-scope-v1.cc:30:1 has some indirect sub-type changes: + parameter 1 of type 'Sm*' has sub-type changes: + in pointed to type 'struct Sm' at test49-var-scope-v1.cc:6:1: + type size hasn't changed + 1 data member insertion: + 'char Sm::wanted[10]', at offset 0 (in bits) at test49-var-scope-v1.cc:7:1 + no data member change (1 filtered); + + [C] 'function void reg(Um*)' at test49-var-scope-v1.cc:32:1 has some indirect sub-type changes: + parameter 1 of type 'Um*' has sub-type changes: + in pointed to type 'union Um' at test49-var-scope-v1.cc:16:1: + type size changed from 160 to 256 (in bits) + no data member change (1 filtered); + +1 Changed variable: + + [C] 'char N::padding[1]' was changed to 'char N::padding[2]' at test49-var-scope-v1.cc:22:1: + size of symbol changed from 1 to 2 + type of variable changed: + type name changed from 'char[1]' to 'char[2]' + array type size changed from 8 to 16 + array type subrange 1 changed length from 1 to 2 + diff --git a/tests/data/test-diff-suppr/test49-var-scope-suppr.txt b/tests/data/test-diff-suppr/test49-var-scope-suppr.txt new file mode 100644 index 00000000..04cc3a98 --- /dev/null +++ b/tests/data/test-diff-suppr/test49-var-scope-suppr.txt @@ -0,0 +1,3 @@ +[suppress_variable] + name_regexp = padding + scope_name_regexp = ^.m$ diff --git a/tests/data/test-diff-suppr/test49-var-scope-v0.cc b/tests/data/test-diff-suppr/test49-var-scope-v0.cc new file mode 100644 index 00000000..e1de3825 --- /dev/null +++ b/tests/data/test-diff-suppr/test49-var-scope-v0.cc @@ -0,0 +1,30 @@ +struct S { + char padding[20]; +}; + +struct Sm { + char padding[20]; +}; + +union U { + char padding[20]; + int x; +}; + +union Um { + char padding[20]; + int x; +}; + +namespace N { + char padding[1]; +}; + +namespace Nm { + char padding[1]; +}; + +void reg(S*) { } +void reg(Sm*) { } +void reg(U*) { } +void reg(Um*) { } diff --git a/tests/data/test-diff-suppr/test49-var-scope-v0.o b/tests/data/test-diff-suppr/test49-var-scope-v0.o new file mode 100644 index 0000000000000000000000000000000000000000..c98fd25033e371c23a49792293d83dd72fa7f723 GIT binary patch literal 4136 zcmbtXUuaup6hB{X^QXOSk~Y(}c1jA$95q=}%erjqmRZ+7IKr?*u{heBf}Zo;@4NZ(Em6S(x!*azbAIPL z_uTJKd-eQ<)c|84g~1uH(Gb{dATJns19guV61;E$ z9mC;|&=J8Ouo3PU?f^463L`_on$w1Gfl@@ncl^kh9~t)|c@1~b&uaiM^S~13ZwQRy z57;3-10{g5@9{CrOCi(DH-Jr=No;@@I;Dih5bcIWUAqG85&l|4C6Ql&eYl0OF?s~v ztwKEV2|7K(!vhHK5P=>Zfg0oy4i6xFK?HhugtAYV89?}n2=wp>DI@?JXOBe_rV*Wq zjwvixDa27Ta#}ElU3Z{7m(*T4g zHF`v)V*}BQ$ZP;3f{AT5v&igDJG$sk>qIvaXQxk3<6$@fn6cmqPSsRG(r5m6Z_7)e{=%FrYA4G)~u>sT~#{P(o;gfGdY#xKQ-D)pp zQwNrtYL%LMcIse0T`F;QFbc!oG{k?11>@I(vE|_SwP1AF*8>Ko8<1ps>q%Tz7c-k6 zwF1hY$h|GO{N;{eaBv|*pBViN@!Ycsk+=baXfW=F`23) zQ@d8{ic_k(O{clvO6@zBYfd@Uwu*JT1+ccVax(e&({{0DIbP;eCVNzygSQLdkQmu3 z&jRbj+01A-aud6J7;*ZM@J)3t8hto>lb{Uy=kH_+2h3~>tUJ_C9-w(mB8pRm3y6$6H)+A|vdG1UWOHDkSi zunfR$-A4@Xi0tv;rh{?iy<|s@2wd87Tta+8=^Ou+h@BI-wB>j~;1A+RB5}HFD9Azn)kA>U(oP5Vee@8RbjuZ;WE$L1MtrW;P(dLKWg~8u>TYz zy|20|;{>3{e#&|#HT-!m42}q#26YuLY4H^ie@cm`KFMsZuEooBc}t657I|)K z@pAp%(c&jX{5M*>fc-T_eF#br$GFgmrmhotR_b_C#8a#qpVM$z=Z=QU_kB~tXN3*v z)|Ccc3^=x3Z>&nKZFky`E?aF2(#2K_(ym>%(q+52U)ioXyG`_Q-6|Gc`=G~dxt3GG zyGEtdYmO}`I0d9j%|^q<+cNDm+jbf+yHu-fm0l7gySlyWS`Awk`v1lmq*HIzy-yi) z^gz-7&Y)OTs9%KXH$q(vQgoe9=PT|1L45VwQW~9~>4Q<6oc~vipqc#r=>LTrkreq< zTupnT09v0EC(kOnFGy&A=s5}B5xgJBP(+Wth&~;o@O9CUd4)@=9{UVp{pz2h4Td85 zR6o6vUVj-eJtsoY5%XJQD5A%1qp#om>tcS{Cz_wmbAA3j#3-k%6ICwXiuviEAywr3 za(oq?{rI0zQ7E*&RFOP6UPq_Y)wsKYuVSR@v9|=KuhZ`Y{{t<7e18eQhYE$}Bp}8e z(Z3yGQ~&55kbK!!sz;xjC>?RQek}^7dz2tF&A%I1|320i30^GBk`jk|sAOl0Nc=pDxXK%dy;O51<|Elha0dz@jQkDqy zW?pzw(_;&;O^DQ?+KA*NRwsuLT^{R;s=metRgd=xR0KYIT$gA zG-ekLS26|I^B}g{dlBfP#A1i!Q3B{sA=)GRNv1tt1uR_I4nAO$7Rc1e(2&u%`pzHzLp{B)lFG9_c_B#+@)g zpOBD60*FEJSSn*%snOJ+7)qpv(VG!g(zcJ;Pum4sWN;Dt!AgbZT;?bau}g@#40+;X zFqb)qNcW>)+b1v$Kw{XSCv`f`HJVAz2GFaRIL&qena#3&6H!?|HJ3R#a(o1rYXZ(> zVmonea^mC&hx@YgQ#(I-W`w#m5XWv&Py^kG}aM5S5Ggvw)^Z zh(A;P_?5>MXIntvHG+vkcGInA8^zkXm)*?gigXfiLQ>EZmO;bXE#m2`@qx+s;MI6) zGSXufrdzu36z-q%`31P_`hi#Kl$GNaVXfwuoejTHDf?c@p_8C|<9OhfF3jiWkF2|; zQpGQyCUbngs@t;KI3AsYL3JHeAQV^KI-HqbbY>15$WJ(h+;}eUj33Gu^7%srCtD3R zd~dSs`Ch$JMANMoS0}fQ9?l+~aI$45yW%#k_{G(F&97}VvK#*8ieJhG?(&+~05~%< zbIjRy%3H3uewaC%FYGsJ$XSDJVFTig7eQ>L#b{q5c@yV)H{$dokwF=J! z0K~|5oSZ0~{4a;@j{dj)!LM8oHP|)0Q~&tuAqN7uxb0xXqm9wxx6sp0EOl#}J_}lo zcjUG}Uc!;hiiH<+JF%k6s?}>je=DFY3-EBa5i7h?TReF5U{D(ukNgRRv(4=h#D}zb zlWU3Cw8Giu_MF0ZU`xXIOQ=(xF^M+u&WG#2oEki%EhjBJxQh{phe`>rOfTB3lS{rj z*b+{w@d5rw2E!)8+1_s8SCowxpcL9+Ja+`_muQ$@zuyWPPIak1$!yp}b>C6;ih++S zn+M4KzpilRG0ypUe2jD4S4Muuf9ik_sH0@&*@U{ZTmE6FE z+;XDYOd8_tigcP=UKh?br4;k86N=S^=8G`%1zFRBG~MLW zeN6X7C%*Z=g2~S#*TkqC*WaUWjyAD0KB_T=KV5!QPub^+Py3VN_->>3g=7+c?drd; zc)!c2=EsaZ-+^CO6LP3Ym72+a4$*e~-%=ML^Qr$dB(wh|#LS!s!56B&ONJ(9>=OFg z)xV?a^E^>~y8q4k>xfZK?kB2JEJ}**uFvh8=xN9QMe%8W>B2m2uc4E5J)&ykBZuQceEfK;cIJ literal 0 HcmV?d00001 diff --git a/tests/test-diff-suppr.cc b/tests/test-diff-suppr.cc index abc1b3d7..fe0c2ef8 100644 --- a/tests/test-diff-suppr.cc +++ b/tests/test-diff-suppr.cc @@ -2048,6 +2048,16 @@ InOutSpec in_out_specs[] = "data/test-diff-suppr/libtest48-soname-abixml-report-1.txt", "output/test-diff-suppr/libtest48-soname-abixml-report-1.txt" }, + { + "data/test-diff-suppr/test49-var-scope-v0.o", + "data/test-diff-suppr/test49-var-scope-v1.o", + "", + "", + "data/test-diff-suppr/test49-var-scope-suppr.txt", + "--no-default-suppression", + "data/test-diff-suppr/test49-var-scope-report.txt", + "output/test-diff-suppr/test49-var-scope-report.txt" + }, // This should be the last entry {NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL} };