This is an archive of the discontinued LLVM Phabricator instance.

Differential D112075

Use safe yaml load_all
ClosedPublic

Authored by kongyi on Oct 19 2021, 7:21 AM.

Details

Reviewers
anemet
serge-sans-paille
Commits
rG1123e03a9d94: [opt-viewer] Use safe yaml load_all

Diff Detail

Event Timeline

kongyi requested review of this revision.Oct 19 2021, 7:21 AM
kongyi created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptOct 19 2021, 7:21 AM
Herald added a subscriber: llvm-commits. · View Herald Transcript
Harbormaster completed remote builds in B129548: Diff 380687.Oct 19 2021, 8:11 AM
kongyi added a reviewer: serge-sans-paille.Oct 20 2021, 5:24 PM
serge-sans-paille accepted this revision.Oct 20 2021, 10:29 PM

LGTM, I wonder what triggered that change?

This revision is now accepted and ready to land.Oct 20 2021, 10:29 PM
kongyi added a comment.Oct 20 2021, 10:55 PM
In D112075#3076966, @serge-sans-paille wrote:

LGTM, I wonder what triggered that change?

This was discovered by our internal security audit.

Closed by commit rG1123e03a9d94: [opt-viewer] Use safe yaml load_all (authored by kongyi). · Explain WhyOct 20 2021, 11:00 PM
This revision was automatically updated to reflect the committed changes.
kongyi added a commit: rG1123e03a9d94: [opt-viewer] Use safe yaml load_all.
shchenz added a subscriber: shchenz.Nov 1 2021, 1:14 AM

Hi @kongyi , this breaks opt-viewer.py usage on the AIX platform.

opt-viewer/opt-viewer.py  test.opt.yaml --demangler cat
For faster parsing, you may want to install libYAML for PyYAML
Reading YAML files...
        1 of 1multiprocessing.pool.RemoteTraceback: 
"""
Traceback (most recent call last):
  File "/opt/freeware/lib64/python3.7/multiprocessing/pool.py", line 121, in worker
    result = (True, func(*args, **kwds))
  File "/opt/freeware/lib64/python3.7/multiprocessing/pool.py", line 44, in mapstar
    return list(map(*args))
  File "/home/czhengsz/llvm/dev/llvm-project/llvm/tools/opt-viewer/optpmap.py", line 25, in _wrapped_func
    return func(argument, filter_)
  File "/home/czhengsz/llvm/dev/llvm-project/llvm/tools/opt-viewer/optrecord.py", line 277, in get_remarks
    docs = yaml.safe_load_all(f, Loader=Loader)
TypeError: safe_load_all() got an unexpected keyword argument 'Loader'
"""
kongyi added a comment.Nov 1 2021, 1:36 AM
In D112075#3099663, @shchenz wrote:

Hi @kongyi , this breaks opt-viewer.py usage on the AIX platform.

opt-viewer/opt-viewer.py  test.opt.yaml --demangler cat
For faster parsing, you may want to install libYAML for PyYAML
Reading YAML files...
        1 of 1multiprocessing.pool.RemoteTraceback: 
"""
Traceback (most recent call last):
  File "/opt/freeware/lib64/python3.7/multiprocessing/pool.py", line 121, in worker
    result = (True, func(*args, **kwds))
  File "/opt/freeware/lib64/python3.7/multiprocessing/pool.py", line 44, in mapstar
    return list(map(*args))
  File "/home/czhengsz/llvm/dev/llvm-project/llvm/tools/opt-viewer/optpmap.py", line 25, in _wrapped_func
    return func(argument, filter_)
  File "/home/czhengsz/llvm/dev/llvm-project/llvm/tools/opt-viewer/optrecord.py", line 277, in get_remarks
    docs = yaml.safe_load_all(f, Loader=Loader)
TypeError: safe_load_all() got an unexpected keyword argument 'Loader'
"""

Can you try removing the second argument (Loader=Loader) and check if it works?

shchenz added a comment.Nov 1 2021, 1:54 AM
In D112075#3099677, @kongyi wrote:
In D112075#3099663, @shchenz wrote:

Hi @kongyi , this breaks opt-viewer.py usage on the AIX platform.

opt-viewer/opt-viewer.py  test.opt.yaml --demangler cat
For faster parsing, you may want to install libYAML for PyYAML
Reading YAML files...
        1 of 1multiprocessing.pool.RemoteTraceback: 
"""
Traceback (most recent call last):
  File "/opt/freeware/lib64/python3.7/multiprocessing/pool.py", line 121, in worker
    result = (True, func(*args, **kwds))
  File "/opt/freeware/lib64/python3.7/multiprocessing/pool.py", line 44, in mapstar
    return list(map(*args))
  File "/home/czhengsz/llvm/dev/llvm-project/llvm/tools/opt-viewer/optpmap.py", line 25, in _wrapped_func
    return func(argument, filter_)
  File "/home/czhengsz/llvm/dev/llvm-project/llvm/tools/opt-viewer/optrecord.py", line 277, in get_remarks
    docs = yaml.safe_load_all(f, Loader=Loader)
TypeError: safe_load_all() got an unexpected keyword argument 'Loader'
"""

Can you try removing the second argument (Loader=Loader) and check if it works?

No, still fails.

Traceback (most recent call last):
  File "/opt/freeware/lib64/python3.7/multiprocessing/pool.py", line 121, in worker
    result = (True, func(*args, **kwds))
  File "/opt/freeware/lib64/python3.7/multiprocessing/pool.py", line 44, in mapstar
    return list(map(*args))
  File "/home/czhengsz/llvm/dev/llvm-project/llvm/tools/opt-viewer/optpmap.py", line 25, in _wrapped_func
    return func(argument, filter_)
  File "/home/czhengsz/llvm/dev/llvm-project/llvm/tools/opt-viewer/optrecord.py", line 282, in get_remarks
    for remark in docs:
  File "/home/czhengsz/test/optviewer/optviewer_env/lib64/python3.7/site-packages/yaml/__init__.py", line 93, in load_all
    yield loader.get_data()
  File "/home/czhengsz/test/optviewer/optviewer_env/lib64/python3.7/site-packages/yaml/constructor.py", line 45, in get_data
    return self.construct_document(self.get_node())
  File "/home/czhengsz/test/optviewer/optviewer_env/lib64/python3.7/site-packages/yaml/constructor.py", line 55, in construct_document
    data = self.construct_object(node)
  File "/home/czhengsz/test/optviewer/optviewer_env/lib64/python3.7/site-packages/yaml/constructor.py", line 100, in construct_object
    data = constructor(self, node)
  File "/home/czhengsz/test/optviewer/optviewer_env/lib64/python3.7/site-packages/yaml/constructor.py", line 429, in construct_undefined
    node.start_mark)
yaml.constructor.ConstructorError: could not determine a constructor for the tag '!Missed'
kongyi added a comment.Nov 1 2021, 2:14 AM
In D112075#3099690, @shchenz wrote:
In D112075#3099677, @kongyi wrote:
In D112075#3099663, @shchenz wrote:

Hi @kongyi , this breaks opt-viewer.py usage on the AIX platform.

opt-viewer/opt-viewer.py  test.opt.yaml --demangler cat
For faster parsing, you may want to install libYAML for PyYAML
Reading YAML files...
        1 of 1multiprocessing.pool.RemoteTraceback: 
"""
Traceback (most recent call last):
  File "/opt/freeware/lib64/python3.7/multiprocessing/pool.py", line 121, in worker
    result = (True, func(*args, **kwds))
  File "/opt/freeware/lib64/python3.7/multiprocessing/pool.py", line 44, in mapstar
    return list(map(*args))
  File "/home/czhengsz/llvm/dev/llvm-project/llvm/tools/opt-viewer/optpmap.py", line 25, in _wrapped_func
    return func(argument, filter_)
  File "/home/czhengsz/llvm/dev/llvm-project/llvm/tools/opt-viewer/optrecord.py", line 277, in get_remarks
    docs = yaml.safe_load_all(f, Loader=Loader)
TypeError: safe_load_all() got an unexpected keyword argument 'Loader'
"""

Can you try removing the second argument (Loader=Loader) and check if it works?

No, still fails.

Traceback (most recent call last):
  File "/opt/freeware/lib64/python3.7/multiprocessing/pool.py", line 121, in worker
    result = (True, func(*args, **kwds))
  File "/opt/freeware/lib64/python3.7/multiprocessing/pool.py", line 44, in mapstar
    return list(map(*args))
  File "/home/czhengsz/llvm/dev/llvm-project/llvm/tools/opt-viewer/optpmap.py", line 25, in _wrapped_func
    return func(argument, filter_)
  File "/home/czhengsz/llvm/dev/llvm-project/llvm/tools/opt-viewer/optrecord.py", line 282, in get_remarks
    for remark in docs:
  File "/home/czhengsz/test/optviewer/optviewer_env/lib64/python3.7/site-packages/yaml/__init__.py", line 93, in load_all
    yield loader.get_data()
  File "/home/czhengsz/test/optviewer/optviewer_env/lib64/python3.7/site-packages/yaml/constructor.py", line 45, in get_data
    return self.construct_document(self.get_node())
  File "/home/czhengsz/test/optviewer/optviewer_env/lib64/python3.7/site-packages/yaml/constructor.py", line 55, in construct_document
    data = self.construct_object(node)
  File "/home/czhengsz/test/optviewer/optviewer_env/lib64/python3.7/site-packages/yaml/constructor.py", line 100, in construct_object
    data = constructor(self, node)
  File "/home/czhengsz/test/optviewer/optviewer_env/lib64/python3.7/site-packages/yaml/constructor.py", line 429, in construct_undefined
    node.start_mark)
yaml.constructor.ConstructorError: could not determine a constructor for the tag '!Missed'

Okay, I'll revert this change for now.

kongyi added a reverting change: rGc060457ec6ed: Revert "[opt-viewer] Use safe yaml load_all".Nov 1 2021, 2:19 AM
kongyi added a comment.Nov 1 2021, 2:19 AM

Reverted, commit c060457ec6eda640ae75e48d99500712fabe1c42

shchenz added a comment.Nov 1 2021, 3:33 AM
In D112075#3099754, @kongyi wrote:

Reverted, commit c060457ec6eda640ae75e48d99500712fabe1c42

thanks, when you have a new fix, I am glad to verify it
on AIX before you commit.

qiucf added a subscriber: qiucf.Nov 4 2021, 2:31 AM

The YAML test files may contain some tags, here including !Passed !Missed !Failure !AnalysisAliasing !AnalysisFPCommute !Analysis.

I think it's the reason that the safe loader can't find appropriate constructor for these tags.

--- a/llvm/tools/opt-viewer/optrecord.py
+++ b/llvm/tools/opt-viewer/optrecord.py
@@ -235,6 +235,7 @@ class Remark(yaml.YAMLObject):


 class Analysis(Remark):
+    yaml_loader = yaml.SafeLoader
     yaml_tag = '!Analysis'

     @property
@@ -243,14 +244,17 @@ class Analysis(Remark):


 class AnalysisFPCommute(Analysis):
+    yaml_loader = yaml.SafeLoader
     yaml_tag = '!AnalysisFPCommute'


 class AnalysisAliasing(Analysis):
+    yaml_loader = yaml.SafeLoader
     yaml_tag = '!AnalysisAliasing'


 class Passed(Remark):
+    yaml_loader = yaml.SafeLoader
     yaml_tag = '!Passed'

     @property
@@ -259,6 +263,7 @@ class Passed(Remark):


 class Missed(Remark):
+    yaml_loader = yaml.SafeLoader
     yaml_tag = '!Missed'

     @property
@@ -266,6 +271,7 @@ class Missed(Remark):
         return "red"

 class Failure(Missed):
+    yaml_loader = yaml.SafeLoader
     yaml_tag = '!Failure'

 def get_remarks(input_file, filter_=None):
@@ -274,7 +280,7 @@ def get_remarks(input_file, filter_=None):
     file_remarks = defaultdict(functools.partial(defaultdict, list))

     with io.open(input_file, encoding = 'utf-8') as f:
-        docs = yaml.load_all(f, Loader=Loader)
+        docs = yaml.safe_load_all(f)

         filter_e = None
         if filter_:

And I think this isn't related to AIX. I found the same issue on Linux. Try llvm/test/tools/opt-viewer/Inputs/suppress/s.opt.yaml.

Revision Contents

PathSize
llvm/
tools/
opt-viewer/
2 lines

Diff 381148

llvm/tools/opt-viewer/optrecord.py

Show First 20 Lines • Show All 268 Lines • ▼ Show 20 Linesclass Failure(Missed):
yaml_tag = '!Failure' yaml_tag = '!Failure'
def get_remarks(input_file, filter_=None): def get_remarks(input_file, filter_=None):
max_hotness = 0 max_hotness = 0
all_remarks = dict() all_remarks = dict()
file_remarks = defaultdict(functools.partial(defaultdict, list)) file_remarks = defaultdict(functools.partial(defaultdict, list))
with io.open(input_file, encoding = 'utf-8') as f: with io.open(input_file, encoding = 'utf-8') as f:
docs = yaml.load_all(f, Loader=Loader) docs = yaml.safe_load_all(f, Loader=Loader)
filter_e = None filter_e = None
if filter_: if filter_:
filter_e = re.compile(filter_) filter_e = re.compile(filter_)
for remark in docs: for remark in docs:
remark.canonicalize() remark.canonicalize()
# Avoid remarks withoug debug location or if they are duplicated # Avoid remarks withoug debug location or if they are duplicated
if not hasattr(remark, 'DebugLoc') or remark.key in all_remarks: if not hasattr(remark, 'DebugLoc') or remark.key in all_remarks:
▲ Show 20 Lines • Show All 61 Lines • Show Last 20 Lines