Project

General

Profile

Actions

Bug #917

closed

32_7: Error trace visualizer and rerouting

Added by Alexey Khoroshilov over 13 years ago. Updated over 13 years ago.

Status:
Closed
Priority:
High
Category:
Infrastructure
Start date:
03/05/2011
Due date:
% Done:

0%

Estimated time:
Detected in build:
f9bb90c
Platform:
Published in build:
6e2696b

Description

Error trace visualizer does not detect model functions in 32_7.
It looks like rerouting is the reason for that, but may be incomplete comments impact in some way as well.


Files

Actions #1

Updated by Pavel Shved over 13 years ago

  • Status changed from New to Feedback

Are these functions are detected with the other models? And what functions exactly are not detected?

Rerouting makes BLAST think that the functions generated reside on the same lines as their templates. This should not introduce any problems for lock/unlock functions. However, foreach functions (such as ldv_initialize and ldv_check_final_state) may not keep their line numbers, hence they won't be printed in the trace.

To check the problem further, could you please attach the PAX file with such a trace?

Actions #2

Updated by Alexey Khoroshilov over 13 years ago

  • Status changed from Feedback to Open

Pavel Shved wrote:

Are these functions are detected with the other models?

I do not know, since all aspect-based models do not work by another reason (S-01087). The story goes it works for 32_1.

And what functions exactly are not detected?

It is much easier to say which functions are not detected than which functions are detected:
at least, mutex_lock, mutex_unlock, mutex_trylock.
Functions generated by drv-env-gen are detected correctly.

Rerouting makes BLAST think that the functions generated reside on the same lines as their templates. This should not introduce any problems for lock/unlock functions. However, foreach functions (such as ldv_initialize and ldv_check_final_state) may not keep their line numbers, hence they won't be printed in the trace.

That is true. ldv_initialize_XXX and ldv_check_final_state_XXX do not have line numbers and hyperlinks.

To check the problem further, could you please attach the PAX file with such a trace?

I will do it when the current run is finished, but actually the issue is applicable to any unsafe with 32_7.

Actions #4

Updated by Pavel Shved over 13 years ago

Okay, let me state the problem: the functions like "ldv_mutex_unlock_xxxx" are not marked as model functions (with pink color) and do not have comments pop up on mouseover in Error-trace-visualizer. Nevertheless, line numbers for these functions are detected correctly.

As for ldv_initialize maingenerator's funcitons, line numbers that contain their calls aren't shown, but their bodies are there. Anyway, these functions also aren't colored pink.

Therefore, first, we need a list of what is missing from the trace you attached for visualizer to work correctly. And second, BLAST's rerouter should be fixed, as I didn't focus on line numbers when developing it.

Actions #5

Updated by Evgeny Novikov over 13 years ago

Alexey Khoroshilov wrote:

That is true. ldv_initialize_XXX and ldv_check_final_state_XXX do not have line numbers and hyperlinks.

This is so because of they refer to the preprocessed file instead to the original one like mutex_lock_XXX. If you'll put your mouse over them you'll obtain following title:

drivers-media-video-au0828-au0828-video.c.common.i:0

I suggest that this comes from here:
Pavel Shved wrote:

And second, BLAST's rerouter should be fixed, as I didn't focus on line numbers when developing it.

As for ETV bug with treatment of mutex_lock_XXX as a model function it's obvious: since in an error trace it's named mutex_lock_XXX while model function is named simply as mutex_lock:

/* LDV_COMMENT_MODEL_FUNCTION_DEFINITION(name='mutex_unlock') Check that the mutex was locked and unlock it*/ 

they aren't related with each other.

To fix it both model comments (like name='mutex_unlock.*') and ETV (to support regexp of model comments attributes) should be fixed.

Actions #6

Updated by Evgeny Novikov over 13 years ago

  • Category changed from Statistics server to Infrastructure
  • Priority changed from Normal to High

We have put this model (32_7) to the 4th quality level, while its model comments and visualization is disappointing as for users so for us. So increase priority to do it sooner.
Because of model comments fix is cosmetic I relate the issue with ETV.

Actions #7

Updated by Evgeny Novikov over 13 years ago

We'd like to add error trace visualization (including model comments, visualization itself and may be something else) as a criteria to our Nice table.

Actions #8

Updated by Evgeny Novikov over 13 years ago

  • Status changed from Open to Resolved
  • Published in build set to a31e19b

The problem with visualization of rerouting issues was fixed in a31e19b. The live example can be found here for a couple of days. Nice table was slightly updated to meet some error trace visualization requirements.
Wait for Alexey verify it and close. Pavel, please, investigate the problem with ldv_initialize_XXX and open a new bug if so.

Actions #9

Updated by Evgeny Novikov over 13 years ago

  • Status changed from Resolved to Open

I think that the fix is incomplete, since if we encounter a function call like mutex_lock_killable... it will be incorrectly associated both with mutex_lock.* and mutex_lock_killable.*. I suppose a solution: to associate names beginning with the longest pattern and to stop on a first matching found.

Actions #10

Updated by Pavel Shved over 13 years ago

Eugene Novikov wrote:

I think that the fix is incomplete, since if we encounter a function call like mutex_lock_killable... it will be incorrectly associated both with mutex_lock.* and mutex_lock_killable.*. I suppose a solution: to associate names beginning with the longest pattern and to stop on a first matching found.

Alternatively, you may match the first pattern encountered in the model file instead of the longest. :) And move mutex_lock_TEMPLATE to the end of the file. This will make things under more control.

Actions #11

Updated by Evgeny Novikov over 13 years ago

I remember that the given approach of manual sorting was failed with regression test sets. But matching by the longest pattern also isn't good, since it'll give incorrect results when patterns will have the same length (another collision will arise when a variable will have the same name as a pattern end). So I think we need to develop some another solution indeed...

Actions #12

Updated by Alexey Khoroshilov over 13 years ago

I believe replacing pattern 'mutex_xxx_TEMPLATE' by 'mutex_xxx__TEMPLATE' fixes most real-world issues.
As a supplement ordering by position in a file can be used.

Actions #13

Updated by Pavel Shved over 13 years ago

Alexey Khoroshilov wrote:

I believe replacing pattern 'mutex_xxx_TEMPLATE' by 'mutex_xxx__TEMPLATE' fixes most real-world issues.
As a supplement ordering by position in a file can be used.

I recall that rerouting in BLAST requires an underscore character in front of the TEMPLATE placeholder. Don't know if it works with two of them. Still, it is worth trying.

Actions #14

Updated by Evgeny Novikov over 13 years ago

Nevertheless the issue is still important. I think that it would be great if blast say in additional comment where a called function is defined (I believe that it knows it).

Actions #15

Updated by Evgeny Novikov over 13 years ago

  • Status changed from Open to Closed
  • Published in build changed from a31e19b to 6e2696b

This is completely fixed in 6e2696b. Here is a real example (that demonstrates that simple cases work) and here is an artificial example based on the first one (it demonstrates complex cases).
Also I removed '.*' from model comments function names because of x =~ /a/ === x =~ /a.*/ in fact.

Actions

Also available in: Atom PDF