Feature #7925
closedInclude callstacks into call forests in case when non-entering function edges have notes and warnings
Added by Anton Vasilyev over 7 years ago. Updated over 7 years ago.
100%
Description
call_forests_compare report that current traces are same:
http://ldvstore:8998/reports/unsafe/73899/
http://ldvstore:8998/reports/unsafe/36530/
Whereas call stacks to error are different:
mmc_blk_probe() -> mcc_cleanup_queue() -> ldv_free()
mmc_bkl_open() -> mcc_blk_put() -> ldv_free()
Is it require to add something into witness to avoid such miss comparison or fix call_forests_compare?
Files
| error-trace.json (386 KB) error-trace.json | Pavel Andrianov, 02/01/2017 11:36 AM |
Updated by Vladimir Gratinskiy over 7 years ago
You should change global job role (or set "Observer" role for me). Now I don't have an access to these reports.
Updated by Vladimir Gratinskiy over 7 years ago
Maybe you want to use another error trace convertion and comparison functions ("call_stack" for example). "call_forests_compare" contains call trees with leaves that have "note". Your error traces don't have at least one note.
Updated by Anton Vasilyev over 7 years ago
"callstack_compare" can't associate mark on http://ldvstore:8998/reports/unsafe/73899/ error trace even if mark was created exactly on this trace.
Is it require to add "note" at last error edge into witness also for callstack comparator?
Updated by Vladimir Gratinskiy over 7 years ago
Anton Vasilyev wrote:
"callstack_compare" can't associate mark on http://ldvstore:8998/reports/unsafe/73899/ error trace even if mark was created exactly on this trace.
You should create new mark with "call_stack" convert and "callstack_compare" comparison functions.
Is it require to add "note" at last error edge into witness also for callstack comparator?
No, callstack comparator needs only warning.
Updated by Evgeny Novikov over 7 years ago
- Priority changed from Normal to Immediate
Please, treat edges with warnings exactly as edges with notes. Before we didn't have such issues since there always were edges with notes under edges with warnings.
Updated by Vladimir Gratinskiy over 7 years ago
Evgeny Novikov wrote:
Please, treat edges with warnings exactly as edges with notes. Before we didn't have such issues since there always were edges with notes under edges with warnings.
It will not fix issue for this error traces as warning is not in enter function edge. So, again, either change functions, or fix error traces.
Updated by Evgeny Novikov over 7 years ago
- Tracker changed from Bug to Feature
- Subject changed from Traces miss comparison to Include callstacks into call forests in case when non-entering function edges have notes and warnings
Anton decided that it will be enough to include into converted error traces callstacks leading to non-entering function edges have notes and warnings. I hope that it can be quite simply done.
Please, also update #7919 if it misses this. Then I will merge, review and deploy both.
Updated by Vladimir Gratinskiy over 7 years ago
Fixed in branch "forests_with_callbacks".
Updated by Evgeny Novikov over 7 years ago
- Status changed from New to Open
I updated ldvstore, but for error traces specified in the issue description it doesn't work.
Updated by Vladimir Gratinskiy over 7 years ago
- Status changed from Open to Feedback
Evgeny Novikov wrote:
I updated ldvstore, but for error traces specified in the issue description it doesn't work.
It means that converted error traces are in cache. To clear it you need to execute in Python shell:
from marks.models import ErrorTraceConvertionCache
ErrorTraceConvertionCache.objects.all().delete()
Or you can restart the job decision as cache cleared when reports are deleted.
I've just tested these error traces and didn't find any problems.
Updated by Evgeny Novikov over 7 years ago
- Status changed from Feedback to Closed
This helped. Should we propose to extend manager tools or clear cache automatically on each functions update?
Updated by Vladimir Gratinskiy over 7 years ago
Evgeny Novikov wrote:
This helped. Should we propose to extend manager tools or clear cache automatically on each functions update?
Functions bodies are not checked now so clearing cache automatically can't be done. Manager tools are for clearing unused data left after some bridge work. So clearing this cache should be done by migrations.
Updated by Vladimir Gratinskiy over 7 years ago
I've added the new migration that deletes needed cache.
Updated by Evgeny Novikov over 7 years ago
Vladimir Gratinskiy wrote:
Evgeny Novikov wrote:
This helped. Should we propose to extend manager tools or clear cache automatically on each functions update?
Functions bodies are not checked now so clearing cache automatically can't be done. Manager tools are for clearing unused data left after some bridge work. So clearing this cache should be done by migrations.
But during population you track somehow what functions changed and report this. Why you can't clear corresponding caches automatically? If this is hard, you can clear all corresponding caches after each population which is performed quite rare. Migrations will be a bit hard to maintain because of models don't changed and one should keep in mind that after each function change it is necessary to create corresponding migration which is also nontrivial. Perhaps functions should be a part of models but this is another story.
Updated by Pavel Andrianov over 7 years ago
- File error-trace.json error-trace.json added
In the attached error trace mutex_lock does not appear in converted error trace. Seems, it also should be considered when error traces are compared (call_forests_compare).
Updated by Vladimir Gratinskiy over 7 years ago
Evgeny Novikov wrote:
Vladimir Gratinskiy wrote:
Evgeny Novikov wrote:
This helped. Should we propose to extend manager tools or clear cache automatically on each functions update?
Functions bodies are not checked now so clearing cache automatically can't be done. Manager tools are for clearing unused data left after some bridge work. So clearing this cache should be done by migrations.
But during population you track somehow what functions changed and report this. Why you can't clear corresponding caches automatically? If this is hard, you can clear all corresponding caches after each population which is performed quite rare. Migrations will be a bit hard to maintain because of models don't changed and one should keep in mind that after each function change it is necessary to create corresponding migration which is also nontrivial. Perhaps functions should be a part of models but this is another story.
During population I track just names as only names are saved in DB. How you think I can check if body changed? I need to collect also bodies of all functions called inside convert function, and bodies of functions called inside these functions and so on. Also I need to collect content of classes. But I need to restrict depth of checking by bridge functions and don't check any library functions.
And I didn't change convert function, I changed method of classes which called somewhere deep in callstack.
Updated by Evgeny Novikov over 7 years ago
- Status changed from Closed to Open
Please, see at https://forge.ispras.ru/issues/7925#note-16.
Updated by Evgeny Novikov over 7 years ago
Vladimir Gratinskiy wrote:
Evgeny Novikov wrote:
Vladimir Gratinskiy wrote:
Evgeny Novikov wrote:
This helped. Should we propose to extend manager tools or clear cache automatically on each functions update?
Functions bodies are not checked now so clearing cache automatically can't be done. Manager tools are for clearing unused data left after some bridge work. So clearing this cache should be done by migrations.
But during population you track somehow what functions changed and report this. Why you can't clear corresponding caches automatically? If this is hard, you can clear all corresponding caches after each population which is performed quite rare. Migrations will be a bit hard to maintain because of models don't changed and one should keep in mind that after each function change it is necessary to create corresponding migration which is also nontrivial. Perhaps functions should be a part of models but this is another story.
During population I track just names as only names are saved in DB. How you think I can check if body changed? I need to collect also bodies of all functions called inside convert function, and bodies of functions called inside these functions and so on. Also I need to collect content of classes. But I need to restrict depth of checking by bridge functions and don't check any library functions.
And I didn't change convert function, I changed method of classes which called somewhere deep in callstack.
Now I understand why converting and comparing changes after source code update even without population. But I can't understand why you can't clear caches when populating. Let's clear these caches for each population independently on whether some new functions were created or some existing functions were removed.
Updated by Vladimir Gratinskiy over 7 years ago
- Due date set to 02/01/2017
- Status changed from Open to Resolved
- % Done changed from 0 to 100
Pavel Andrianov wrote:
In the attached error trace mutex_lock does not appear in converted error trace. Seems, it also should be considered when error traces are compared (call_forests_compare).
Because there was no enter function under note. I fixed it in "forests_with_callbacks" so functions that contains notes are considered as model functions as functions with warnings.
Now you need population to clear cache with converted error traces.
Updated by Evgeny Novikov over 7 years ago
- Status changed from Resolved to Closed
I merged the branch to master again in 1ed126a. One needs to populate databases to take changes.
BTW, most likely there is a tricky issue. If you change somehow existing comparison criterion this doesn't affect already established bindings between unsafes and marks. This isn't a bug at all, since these bindings were established, analyzed and confirmed by users before. One can't just recalculate them when populating/migrating databases, since after that you can miss some of them or add new ones. But further users can be surprised why some bindings exist although updated/fixed comparison criterion shouldn't behave in such the way. First users should understand this and take care or this themselves. Second, if I remember properly, we designed functions so that if there is some change, then it should take new names, e.g. function_2, function_3, etc. It can be a bit inconvenient for developers and likely we will always forget about this rule but users will explicitly know why some bindings exist and some doesn't.
Updated by Vladimir Gratinskiy over 7 years ago
Evgeny Novikov wrote:
I merged the branch to master again in 1ed126a. One needs to populate databases to take changes.
BTW, most likely there is a tricky issue. If you change somehow existing comparison criterion this doesn't affect already established bindings between unsafes and marks. This isn't a bug at all, since these bindings were established, analyzed and confirmed by users before. One can't just recalculate them when populating/migrating databases, since after that you can miss some of them or add new ones. But further users can be surprised why some bindings exist although updated/fixed comparison criterion shouldn't behave in such the way. First users should understand this and take care or this themselves. Second, if I remember properly, we designed functions so that if there is some change, then it should take new names, e.g. function_2, function_3, etc. It can be a bit inconvenient for developers and likely we will always forget about this rule but users will explicitly know why some bindings exist and some doesn't.
Population deletes all functions that doesn't exist. It would delete all marks that have links to these functions. And deleting marks would delete all connections with reports.
Updated by Evgeny Novikov over 7 years ago
Vladimir Gratinskiy wrote:
Evgeny Novikov wrote:
I merged the branch to master again in 1ed126a. One needs to populate databases to take changes.
BTW, most likely there is a tricky issue. If you change somehow existing comparison criterion this doesn't affect already established bindings between unsafes and marks. This isn't a bug at all, since these bindings were established, analyzed and confirmed by users before. One can't just recalculate them when populating/migrating databases, since after that you can miss some of them or add new ones. But further users can be surprised why some bindings exist although updated/fixed comparison criterion shouldn't behave in such the way. First users should understand this and take care or this themselves. Second, if I remember properly, we designed functions so that if there is some change, then it should take new names, e.g. function_2, function_3, etc. It can be a bit inconvenient for developers and likely we will always forget about this rule but users will explicitly know why some bindings exist and some doesn't.
Population deletes all functions that doesn't exist. It would delete all marks that have links to these functions. And deleting marks would delete all connections with reports.
Good to know too. I hope that everybody understood me properly. I didn't suggest to remove functions - just to add new "versions" of these functions. So all existing marks and bindings will remain but one will see that they were created using old "versions" of functions.
The next question. If there is knowledge with what functions converted (pattern) error traces were created? If yes and they can be deleted when corresponding conversion functions will be deleted, then when one will edit them manually this binding should be broken to avoid deletion of semi-manually created marks.
Updated by Vladimir Gratinskiy over 7 years ago
Evgeny Novikov wrote:
Vladimir Gratinskiy wrote:
Evgeny Novikov wrote:
I merged the branch to master again in 1ed126a. One needs to populate databases to take changes.
BTW, most likely there is a tricky issue. If you change somehow existing comparison criterion this doesn't affect already established bindings between unsafes and marks. This isn't a bug at all, since these bindings were established, analyzed and confirmed by users before. One can't just recalculate them when populating/migrating databases, since after that you can miss some of them or add new ones. But further users can be surprised why some bindings exist although updated/fixed comparison criterion shouldn't behave in such the way. First users should understand this and take care or this themselves. Second, if I remember properly, we designed functions so that if there is some change, then it should take new names, e.g. function_2, function_3, etc. It can be a bit inconvenient for developers and likely we will always forget about this rule but users will explicitly know why some bindings exist and some doesn't.
Population deletes all functions that doesn't exist. It would delete all marks that have links to these functions. And deleting marks would delete all connections with reports.
Good to know too. I hope that everybody understood me properly. I didn't suggest to remove functions - just to add new "versions" of these functions. So all existing marks and bindings will remain but one will see that they were created using old "versions" of functions.
It is bad as functions are complicated and take a lot of code.