Closed Bug 436342 Opened 16 years ago Closed 6 years ago

Treehydra: Look for bad sql in mozilla

Categories

(Developer Infrastructure :: Source Code Analysis, defect)

x86
All
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED WONTFIX

People

(Reporter: taras.mozilla, Unassigned)

References

Details

Attachments

(2 files, 5 obsolete files)

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
Assignee: nobody → vladimir.sukhoy
Status: NEW → ASSIGNED
Attached patch wip (obsolete) — Splinter Review
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.
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.
Attached patch libs compat patch (obsolete) — Splinter Review
Attachment #323030 - Attachment is obsolete: true
Attachment #323186 - Flags: review?(tglek)
Attached patch modd sql.diff (obsolete) — Splinter Review
Modified sql.diff for portability, looks like it works on mac.
Summary: Look for bad sql in mozilla → Treehydra: Look for bad sql in mozilla
Attached patch This works well on linux (obsolete) — Splinter Review
Depends on: 437647
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.
Attachment #323186 - Flags: review?(tglek) → review-
Attached patch v0.1Splinter Review
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.
Assignee: vladimir.sukhoy → sdwilsh
Attachment #323187 - Attachment is obsolete: true
Attachment #324063 - Attachment is obsolete: true
Attached patch libs compat patch v2 (obsolete) — Splinter Review
Fixed both issues with the libs compat patch as requested.
Attachment #323186 - Attachment is obsolete: true
Attachment #324102 - Flags: review?(tglek)
Comment on attachment 324102 [details] [diff] [review]
libs compat patch v2

Oops, nevermind.
Attachment #324102 - Flags: review?(tglek)
The actual patch.
Attachment #324102 - Attachment is obsolete: true
Attachment #324104 - Flags: review?(tglek)
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.
Attachment #324104 - Flags: review?(tglek) → review+
pushed the libs compat patch. case (isGCCApple ...): is no longer necessary in the analysis.
Blocks: 463344
No longer blocks: 463344
Assignee: sdwilsh → dwitte
Blocks: 405920
Reassigning to nobody. If anyone wants to work on this, feel free!
Assignee: dwitte → nobody
Status: ASSIGNED → NEW
Product: Core → Firefox Build System
Let's give up on this.
Nothing for 8 years, this isn't probably critical.
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → WONTFIX
Product: Firefox Build System → Developer Infrastructure
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: