436342 - Treehydra: Look for bad sql in mozilla

Status: RESOLVED WONTFIX

(Developer Infrastructure :: Source Code Analysis, defect)

Description

[(dormant account)]

Not sure what qualifies as inappropriate sql, but I have a script that can dump out sql statements on linux
http://hg.mozilla.org/users/tglek_mozilla.com/index.cgi/dehydra-mq/file/e935d5f002ae/sql.diff

Would like to get that working on mac. i think that means modifying gcc_compat a bit

Comment 1

[Vlad Sukhoy]

Attachment: wip

I quickly (and dirtily) went over tree processing problems I spotted in current test suite. This patch makes the test suite (locks) pass on mac, the problems seen are for the most part known or simple to track ast differences.

Eventually the outcome of this bug should make outparams and other esp-based stuff more feasible on mac as well as this sql thing. I also suspect that MODIFY_EXPR is mac's GIMPLE_MODIFY_STMT but this has to be looked into in more detail.

Comment 2

[(dormant account)]

Thanks for looking into this. That's bad, I underestimated the difficulty of this. I didn't realize there would be so many differences. I wonder if there is even a sane way to do cross-version scripting.

Thanks for looking into it.

Comment 3

[Vlad Sukhoy]

Attachment: libs compat patch

Comment 4

[Vlad Sukhoy]

Attachment: modd sql.diff

Modified sql.diff for portability, looks like it works on mac.

Comment 5

[(dormant account)]

Attachment: This works well on linux

Comment 6

[(dormant account)]

Comment on attachment 323186 [details] [diff] [review]
libs compat patch


> function CALL_EXPR_FN (node) {
>-  return TREE_OPERAND (TREE_CHECK (node, CALL_EXPR), 1)
>+  return TREE_OPERAND (TREE_CHECK (node, CALL_EXPR), isGCCApple ? 0 : 1)
> }
Above should be a constant defined outside of the function. No need to calculate static stuff every call.

>+  case (isGCCApple ? MODIFY_EXPR : GIMPLE_MODIFY_STMT):
Cute use of js syntax here. However most of these changes seem mechanical. I think in this case it's safe to just do

if(isGCCApple) {
this.GIMPLE_MODIFY_STMT = MODIFY_EXPR
... the other renames like POINTER_PLUS_EXPR go here
}

This gives linux scripts a higher chance of working on osx out of the box. It's a bit of hack, but I think it's our best bet.

I think the rest of the patch is excellent as usual. Sorry it took so long to review.

Comment 7

[Shawn Wilsher :sdwilsh]

Attachment: v0.1

Early work on doing what we already dynamically check at runtime in debug builds and issue an NS_WARNING for.  Completely untested at the moment.

Comment 8

[Vlad Sukhoy]

Attachment: libs compat patch v2

Fixed both issues with the libs compat patch as requested.

Comment 9

[Vlad Sukhoy]

Comment on attachment 324102 [details] [diff] [review]
libs compat patch v2

Oops, nevermind.

Comment 10

[Vlad Sukhoy]

Attachment: actual libs compat patch v2

The actual patch.

Comment 11

[(dormant account)]

Comment on attachment 324104 [details] [diff] [review]
actual libs compat patch v2

>+var call_expr_arg_iterator = isGCCApple ?
>+  function (node) {
>+    let arg_holder = TREE_OPERAND(node, 1);
>+    while (arg_holder) {
>+      yield TREE_VALUE(arg_holder);
>+      arg_holder = TREE_CHAIN(arg_holder);
>+    }

Please rewrite this into the 
for(let arg_holder= ...;arg_holder;arg_holder =...) 
form and commit.

Comment 12

[Vlad Sukhoy]

pushed the libs compat patch. case (isGCCApple ...): is no longer necessary in the analysis.

Comment 13

[[e-mail]]

Reassigning to nobody. If anyone wants to work on this, feel free!

Comment 14

[Sylvestre Ledru [:Sylvestre]]

Let's give up on this.
Nothing for 8 years, this isn't probably critical.