Have not done with reviewing. This is the first batch.
David http://codereview.appspot.com/5303083/diff/1/gcc/passes.c File gcc/passes.c (right): http://codereview.appspot.com/5303083/diff/1/gcc/passes.c#newcode1423 gcc/passes.c:1423: NEXT_PASS (pass_tsan); Move this to the same place as asan. Otherwise TARGET_MEM_REF won't be handled. http://codereview.appspot.com/5303083/diff/1/gcc/tree-tsan.c File gcc/tree-tsan.c (right): http://codereview.appspot.com/5303083/diff/1/gcc/tree-tsan.c#newcode56 gcc/tree-tsan.c:56: The instrumentation module mainintains shadow call stacks s/mainitains/maintains/ http://codereview.appspot.com/5303083/diff/1/gcc/tree-tsan.c#newcode60 gcc/tree-tsan.c:60: Instrumentation for shadow stack maintainance is as follows: s/maintainance/maintenance/ http://codereview.appspot.com/5303083/diff/1/gcc/tree-tsan.c#newcode94 gcc/tree-tsan.c:94: #define RTL_STACK "__tsan_shadow_stack" Please change RTL_ prefix to TSAN_. It is confusing to use RTL_ http://codereview.appspot.com/5303083/diff/1/gcc/tree-tsan.c#newcode100 gcc/tree-tsan.c:100: enum tsan_ignore_e better to be tsan_ignore_type or tsan_ignore_kind. http://codereview.appspot.com/5303083/diff/1/gcc/tree-tsan.c#newcode110 gcc/tree-tsan.c:110: enum bb_state_e A new empty line is needed. Same for other comments leading a decl, or function. http://codereview.appspot.com/5303083/diff/1/gcc/tree-tsan.c#newcode110 gcc/tree-tsan.c:110: enum bb_state_e bb_state_e -->bb_state http://codereview.appspot.com/5303083/diff/1/gcc/tree-tsan.c#newcode119 gcc/tree-tsan.c:119: struct bb_data_t _t suffix is better removed. Same for other types with _t suffix. http://codereview.appspot.com/5303083/diff/1/gcc/tree-tsan.c#newcode161 gcc/tree-tsan.c:161: tree __attribute__((weak)) Explain this. http://codereview.appspot.com/5303083/diff/1/gcc/tree-tsan.c#newcode169 gcc/tree-tsan.c:169: extern __thread void **__tsan_shadow_stack; */ Need two white space before */. Same for other instances. http://codereview.appspot.com/5303083/diff/1/gcc/tree-tsan.c#newcode182 gcc/tree-tsan.c:182: Better use varpool_get_node interface. http://codereview.appspot.com/5303083/diff/1/gcc/tree-tsan.c#newcode186 gcc/tree-tsan.c:186: TREE_STATIC (def) = 1; Why mark TREE_STATIC (def) = 1? Should the variable be defined in tsan library? http://codereview.appspot.com/5303083/diff/1/gcc/tree-tsan.c#newcode189 gcc/tree-tsan.c:189: DECL_TLS_MODEL (def) = decl_default_tls_model (def); Check if targetm.have_tls -- though for those target, tsan won't be used. http://codereview.appspot.com/5303083/diff/1/gcc/tree-tsan.c#newcode200 gcc/tree-tsan.c:200: { Refactor the code so that it can be shared with the above one. http://codereview.appspot.com/5303083/diff/1/gcc/tree-tsan.c#newcode228 gcc/tree-tsan.c:228: { The name of the function is very confusing. Change it to get_tsan_mop_handler_decl or something like that. http://codereview.appspot.com/5303083/diff/1/gcc/tree-tsan.c#newcode251 gcc/tree-tsan.c:251: /* Adds new ignore definition to the global list */ Add documentation on function parameters (in upper case) such as " TYPE is the ignore type, and NAME is the name of the function to be ignored. If there is return value, document it too. http://codereview.appspot.com/5303083/diff/1/gcc/tree-tsan.c#newcode257 gcc/tree-tsan.c:257: desc = (struct tsan_ignore_desc_t*)xmalloc (sizeof (*desc)); Use XCNEW to clear. http://codereview.appspot.com/5303083/diff/1/gcc/tree-tsan.c#newcode264 gcc/tree-tsan.c:264: /* Checks as to whether identifier 'str' matches template 'templ'. Use STR instead of 'str'. 'templ' --> TEMPL. http://codereview.appspot.com/5303083/diff/1/gcc/tree-tsan.c#newcode291 gcc/tree-tsan.c:291: if (spos == NULL) Move the check up right after spos is computed. http://codereview.appspot.com/5303083/diff/1/gcc/tree-tsan.c#newcode349 gcc/tree-tsan.c:349: printf ("failed to open ignore file '%s'\n", flag_tsan_ignore); Use error (..) http://codereview.appspot.com/5303083/diff/1/gcc/tree-tsan.c#newcode360 gcc/tree-tsan.c:360: if (line [sz-1] == '\r' || line [sz-1] == '\n') sz-1 --> sz - 1 Change other instances http://codereview.appspot.com/5303083/diff/1/gcc/tree-tsan.c#newcode391 gcc/tree-tsan.c:391: src_name = expand_location(cfun->function_start_locus).file; space before ( http://codereview.appspot.com/5303083/diff/1/gcc/tree-tsan.c#newcode413 gcc/tree-tsan.c:413: static const char * Missing documentation. http://codereview.appspot.com/5303083/diff/1/gcc/tree-tsan.c#newcode443 gcc/tree-tsan.c:443: tree rtl_stack; Do not use rtl_ prefix. Same for other instances. http://codereview.appspot.com/5303083/diff/1/gcc/tree-tsan.c#newcode459 gcc/tree-tsan.c:459: s = NULL; MODIFY_EXPR? directly use gimple_build_assign. http://codereview.appspot.com/5303083/diff/1/gcc/tree-tsan.c#newcode725 gcc/tree-tsan.c:725: This is wrong. SSA_NAME expr should be skipped. http://codereview.appspot.com/5303083/diff/1/gcc/tree-tsan.c#newcode730 gcc/tree-tsan.c:730: { remove {} Same for other one liners. Many conditions should be merged using || http://codereview.appspot.com/5303083/diff/1/gcc/tree-tsan.c#newcode751 gcc/tree-tsan.c:751: && TREE_STATIC (expr) == 0) If you want to skip local variables, check DECL_EXTERNAL attribute. http://codereview.appspot.com/5303083/diff/1/gcc/tree-tsan.c#newcode783 gcc/tree-tsan.c:783: tree field = expr->exp.operands [1]; Do not directly reference tree's member like this. Use TREE_OPERAND http://codereview.appspot.com/5303083/diff/1/gcc/tree-tsan.c#newcode786 gcc/tree-tsan.c:786: fld_off = field->field_decl.bit_offset->int_cst.int_cst.low; Do not access tree like this. Use DECL_FIELD_OFFSET etc. http://codereview.appspot.com/5303083/diff/1/gcc/tree-tsan.c#newcode844 gcc/tree-tsan.c:844: } Remove code handling arguments. THey should not have mem refs. http://codereview.appspot.com/5303083/diff/1/gcc/tree-tsan.c#newcode854 gcc/tree-tsan.c:854: Remove lhs handling -- no need. http://codereview.appspot.com/5303083/diff/1/gcc/tree-tsan.c#newcode877 gcc/tree-tsan.c:877: } Remove handling of GIMPLE_BIND http://codereview.appspot.com/5303083/diff/1/gcc/tree-tsan.c#newcode1003 gcc/tree-tsan.c:1003: { For topo order traversal, use inverted_post_order_compute and then handling bb s in that order. This will simplify the code a lot. http://codereview.appspot.com/5303083/diff/1/gcc/tree-tsan.c#newcode1141 gcc/tree-tsan.c:1141: { Better way is to walk through all predecessors of EXIT_BLOCK_PTR. http://codereview.appspot.com/5303083/diff/1/gcc/tree-tsan.c#newcode1167 gcc/tree-tsan.c:1167: return 0; No need for the above. http://codereview.appspot.com/5303083/